mantid
mantid copied to clipboard
Error in ISIS SANS GUI when scaling merged reduction with fit while plot results is selected
Original reporter: Steve from the SANS Group at ISIS, on behalf of a facility user
Describe the bug
When performing a merged reduction, if the option to scale using a fit is selected along with the option to plot the results then an error is encountered. Among other things, this prevents outputs being saved to file, if requested.
Using either of these options separately does not cause an issue - i.e. a merged reduction that doesn't use a scale fit but plots the results completes successfully, as does a merged reduction that uses the scale fit but does not plot the results. It's using both together that produces the error.
To Reproduce
- Go to
Interfaces
->SANS
->ISIS SANS
- At the top of the Runs page, load the user file
MaskFile.toml
from the loqdemo folder in the TrainingCourseData. - Load the
batch_mode_reduction.csv
file from the loqdemo folder in the TrainingCourseData. - Navigate to the Settings page from the left hand menu. Under the
General, Scale, Event Slice, Sample
tab change the following:- Set the
Reduction Mode
dropdown at the top tomerged
. - In the
Merged Settings
section at the top, tick theUse fit
checkbox next to theScale
field.
- Set the
- Use the left hand menu to navigate back to the Runs page.
- At the bottom right, tick the
Plot Results
checkbox. - Select the first row in the table and click the
Process Selected
button to reduce it. The reduction will appear to complete successfully (based on message in the messages pane and production of some output workspaces), however nothing is plotted and the row turns blue, indicating an error. Hovering over the row displays this error message:
wrapped C/C++ object of type QAction has been deleted
If the save options are set to save to a file (i.e. if option File
or Both
has been selected at the bottom of the Runs page) then no outputs are saved, so it appears that some of the final stages of the reduction workflow are not completing.
Expected behavior
We have discussed this functionality with the SANS Group and they explained that it's not really used by instrument scientists and rarely used/use is discouraged for facility users. As a result, this should be resolved in any of the following ways according to which is the quickest:
- A: Remove the plot results button but provide an option for this to be set via properties for niche cases where people would still like access to it. If never used, the functionality could eventually be removed (presumably if we did this we would still need to resolve the original issue, or prevent users from encountering it somehow?).
- Size: 1-2. I think if we do this we shouldn't fix the bug. This just gives the user a workaround if they aren't affected by the bug to continue using the plot results checkbox. It should be a softer version of Option B. It'll essentially be deprecating it to tell us if we should remove it or fix it. A simple
if
based on a result from the ConfigService would decide whether or not to hide the checkbox.
- Size: 1-2. I think if we do this we shouldn't fix the bug. This just gives the user a workaround if they aren't affected by the bug to continue using the plot results checkbox. It should be a softer version of Option B. It'll essentially be deprecating it to tell us if we should remove it or fix it. A simple
- B: Remove the plot results button altogether.
- Size: 2 The plot results parameter (determined by the checkbox) goes through the whole call stack for the reduction. If we remove this we'll want to do some thorough testing of the GUI and the ICI to make sure we got all of the calls as we can't rely on catching them from compiler issues thanks to python.
- C: Fix the underlying issue so that the reduction workflow should complete fully without any errors, producing a plot and saving outputs to file, if requested.
- Size: 2-4. I did a bit of debugging. Error seems to occur in
plotfunctions.py
(L:287) when the toolbar's history buttons are updated. I have no idea why this would be the case. Will need more investigation.
- Size: 2-4. I did a bit of debugging. Error seems to occur in
The problem caused quite a bit of confusion and took up several people's time to diagnose, which is why we would like to prevent it being encountered again in future. It is otherwise considered low priority, though.
After discussion with the SANS Group, they have confirmed a preference to go for option A. If after three versions of Mantid there have been no problems then we should remove the checkbox altogether (i.e. complete option B). If the conclusion is that the functionality should be retained then we fix the bug (i.e. complete option C). Implementation of the long-term solution should be done under a separate ticket.
Platform/Version (please complete the following information):
- OS: tested on IDAaaS and Windows
- Mantid Version - at least as far back as version 6.7