mantid icon indicating copy to clipboard operation
mantid copied to clipboard

Error in ISIS SANS GUI when scaling merged reduction with fit while plot results is selected

Open rbauststfc opened this issue 8 months ago • 0 comments

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

  1. Go to Interfaces->SANS->ISIS SANS
  2. At the top of the Runs page, load the user file MaskFile.toml from the loqdemo folder in the TrainingCourseData.
  3. Load the batch_mode_reduction.csv file from the loqdemo folder in the TrainingCourseData.
  4. 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 to merged.
    • In the Merged Settings section at the top, tick the Use fit checkbox next to the Scale field.
  5. Use the left hand menu to navigate back to the Runs page.
  6. At the bottom right, tick the Plot Results checkbox.
  7. 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.
  • 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.

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

rbauststfc avatar Jun 20 '24 11:06 rbauststfc