Better error message when processGroups() fails

[Arturs Bekasovs]

When Algorith:processGroups() fails, we get a message like

Execution of ApplyDeadTimeCorr for group entry 1 failed

If the logging is disabled, there is no way for the user to find out what has gone wrong.

This is what currently happens:

if (!alg->execute())
          throw std::runtime_error("Execution of " + this->name() + " for group entry " + Strings::toString(entry+1) + " failed.");

We should rather catch an exception from alg->execute() and appends the Exception::what() value to the message, so the error becomes something like

Execution of ApplyDeadTimeCorr for group entry 1 failed: Number of good frames in the workspace is zero

[Arturs Bekasovs]

(In #9110) Need improve error messages returned when execution on groups fail first. This is not really informative:

No dead time correction applied: Execution of ApplyDeadTimeCorr for group entry 1 failed

[Owen Arnold]

Sounds like a good move. Personally I would try to propagate the original exception, as well as raising an additional message that this came from group workspace processing.

[Nick Draper]

Added Core keyword as this is a change to the base algorithm class. Discuss your planned change with Martyn or Owen before going ahead.

Tester: Test carefully

[Arturs Bekasovs]

Planned change discussed with Martyn. It was decided to use the approach I've suggested in the description.

[Arturs Bekasovs]

Refs #9129. Test for the error message

Changeset: 90e6723fb18a0522e52f772981d04c8619f50e6a

[Arturs Bekasovs]

Refs #9129. Append original exception message

Changeset: dbda5b1ed339d55fad47ce7f650b9096eb883260

[Arturs Bekasovs]

Tester:

As Nick mentioned in comment:3, this is core, so test carefully.

Code review is crucial here. Please spend a few minutes making sure that my change seems to be harmless.

Verify that the unit test was added and that it seems sensible. I've decided not to check the error message char-by-char, but just check that error message thrown by processGroups() contains the original error message thrown by the algorithm.

Then run the following script first in the current stable version of Mantid and then with the changes merged, and compare the exception error messages:

ws = LoadMuonNexus("MUSR15189")

alg = AlgorithmManager.create("AsymmetryCalc")
alg.setLogging(False)
alg.setRethrows(True)
alg.setProperty("InputWorkspace", "ws")
alg.setProperty("OutputWorkspace", "output")
alg.setProperty("ForwardSpectra", 999)
alg.execute()

[Arturs Bekasovs]

Refs #9129. Better test name and remove debugging statement

Changeset: d9599247484e148e8a83cf3107aa806ba85c8b1f

[Martyn Gigg]

I've tested the script with pass/fail scenarios for the running algorithm and everything seems fine.

I also wrote a simple Python algorithm that just raised an error:

from mantid.api import *
from mantid.kernel import *

class FailingAlg(PythonAlgorithm):
        def PyInit(self):
                self.declareProperty(WorkspaceProperty("Input","",Direction.Input))
                
        def PyExec(self):
                raise ValueError("IncorrectValue")

AlgorithmFactory.subscribe(FailingAlg)

This checks that we catch the errors raised by Python too and indeed it does.

[Martyn Gigg]

Merge remote-tracking branch 'origin/feature/9129_process_groups_better_error_msg'

Full changeset: ​08aac711a8e813a91e771b50d98fef8209ec86f3

[Stuart Campbell]

This ticket has been transferred to github issue ​9972