Skip to content

Make instrument optional in LoadDiffCal#41070

Merged
SilkeSchomann merged 8 commits intomantidproject:mainfrom
peterfpeterson:grouping_ws_noinst
Mar 24, 2026
Merged

Make instrument optional in LoadDiffCal#41070
SilkeSchomann merged 8 commits intomantidproject:mainfrom
peterfpeterson:grouping_ws_noinst

Conversation

@peterfpeterson
Copy link
Member

@peterfpeterson peterfpeterson commented Mar 13, 2026

VULCAN added more detector banks and we are unable to process any of their data, including the previously existing banks, until a new IDF is created. This get past the issue in terms of loading the calibration file.

There is no associated issue, but this is related to EWM15052.

To test:

Pick your favorite diff-cal file and load it. The "data" can be viewed and the right-click menu item for showing the instrument is disabled.


Reviewer

Your comments will be used as part of the gatekeeper process. Comment clearly on what you have checked and tested during your review. Provide an audit trail for any changes requested.

As per the review guidelines:

  • Is the code of an acceptable quality? (Code standards/GUI standards)
  • Has a thorough functional test been performed? Do the changes handle unexpected input/situations?
  • Are appropriately scoped unit and/or system tests provided?
  • Do the release notes conform to the guidelines and describe the changes appropriately?
  • Has the relevant (user and developer) documentation been added/updated?
  • If the PR author isn’t in the mantid-developers or mantid-contributors teams, add a review comment rerun ci to authorize/rerun the CI

Gatekeeper

As per the gatekeeping guidelines:

  • Has a thorough first line review been conducted, including functional testing?
  • At a high-level, is the code quality sufficient?
  • Are the base, milestone and labels correct?

@peterfpeterson peterfpeterson added this to the Release 6.16 milestone Mar 13, 2026
@peterfpeterson peterfpeterson added Diffraction Issues and pull requests related to diffraction Powder Issues and pull requests related to powder diffraction labels Mar 13, 2026
@peterfpeterson peterfpeterson changed the title Grouping ws noinst Make instrument optoinal in LoadDiffCal Mar 13, 2026
@peterfpeterson peterfpeterson changed the title Make instrument optoinal in LoadDiffCal Make instrument optional in LoadDiffCal Mar 16, 2026
@peterfpeterson peterfpeterson marked this pull request as ready for review March 16, 2026 20:21
Copy link
Contributor

@rboston628 rboston628 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think inlining the new constructors is a good idea.
Are we sure there's no risk of nullptr dereference?

@peterfpeterson
Copy link
Member Author

The detids is passed by reference so I don't think you can have a nullptr

@rboston628 rboston628 marked this pull request as draft March 17, 2026 16:58
@rboston628
Copy link
Contributor

Was able to build and run. I ran the following script

file = "/SNS/SNAP/shared/Calibration/Powder/04bd2c53f6bf6754/lite/diffraction/v_0001/diffract_consts_046680_v0001.h5"
LoadDiffCal(Filename = file, WorkspaceName = "lite")
file = "/SNS/SNAP/shared/Calibration/Powder/04bd2c53f6bf6754/native/diffraction/v_0/diffract_consts_default_v0.h5"
LoadDiffCal(Filename = file, WorkspaceName = "difc")
LoadDiffCal(file)

The first uses a SNAPLite diffcal file. The second uses a SNAP diffcal file. The third uses a SNAP diffcal file without a workspace name.

In the SNAPLite case, the following error output:

LoadDiffCal started
Extension ['.nxs.h5'] not found.
Searching for other facility extensions.
The instrument definition filename does not contain _Definition. Your instrument name will be set to: SNAP_46680.lite.nxs.h5
Error in execution of algorithm LoadInstrument:
FileDescriptor() - File 'SNAP_46680.lite.nxs.h5' does not exist
LoadDiffCal successful, Duration 0.12 seconds

In the SNAP cases, the following:

LoadDiffCal started
Either the InstrumentName or Filename property of LoadInstrument most be specified
Error in execution of algorithm LoadInstrument:
Either the InstrumentName or Filename property of LoadInstrument must be specified to load an instrument in ""
LoadDiffCal successful, Duration 2.33 seconds

At the end, the expected workspaces are created. For the case where WorkspaceName is not given, the workspaces are _cal, _group, and _mask.

In all three cases, the matrix workspaces have "Show Instrument" disabled. Selecting to "Show Data" shows the table data. For the calibration tables, double clicking will show the table data as well.

However, double-clicking on any of the matrix workspaces causes a segmentation fault:

Received signal: SIGSEGV
Segmentation fault

@peterfpeterson peterfpeterson marked this pull request as ready for review March 23, 2026 18:04
Copy link
Contributor

@rboston628 rboston628 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-ran the above script. Everything still worked. Double-clicking the workspaces brought up a plot without segfaulting.

@SilkeSchomann SilkeSchomann self-assigned this Mar 24, 2026
@SilkeSchomann SilkeSchomann merged commit b96d616 into mantidproject:main Mar 24, 2026
11 checks passed
peterfpeterson added a commit to peterfpeterson/mantid that referenced this pull request Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Diffraction Issues and pull requests related to diffraction Powder Issues and pull requests related to powder diffraction

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants