Ticket #8878 (closed: duplicate)
NOMAD calibration crashes mantid
Reported by: | Peter Peterson | Owned by: | Wenduo Zhou |
---|---|---|---|
Priority: | critical | Milestone: | Release 3.2 |
Component: | Diffraction | Keywords: | |
Cc: | petersonpf@… | Blocked By: | #9020 |
Blocking: | Tester: | Peter Peterson |
Description (last modified by Wenduo Zhou) (diff)
Run
CalibrateRectangularDetectors(
Instrument="NOM", RunNumber=23234, MaxOffset=.1, CrossCorrelation=False, PeakPositions = [2.0600,1.2615,1.0758,0.8920,0.8186,0.7283,0.6867,0.6307,0.5947,0.5642,0.5441,0.5150,0.4996,0.4768,.4645,.4205,.3916,.3499,.3257,.3117], PeakWindowMax=.1, Binning=[.3,-.0004,3], CrossCorrelation = False, DiffractionFocusWorkspace=False, GroupDetectorsBy="All", FilterBadPulses=True, SaveAs="calibration", OutputDirectory="/tmp")
The script crashes. Hopefully this can be characterized and fixed quickly.
Investigation
- gdb shows that the crash (segmentation fault) comes from algorithm FindPeakBackground;
- In FindPeakBackground::exec(): l0 and n are calculated with X vector of spectrum 0. This seems to cause issue during working with a multiple spectra workspace;
- In FindPeakBackground::estimateBackground(), if the number of background points defined by (i_min, p_min) and (p_max, i_max) are too few, the estimated background may not be correct. For example, if there is only 1 background point in the range, the estimated background is flat, which can be way off. Therefore, there should be a checking mechanism to define the quality of background estimation;
- Zero background is output in some situation though the background may not be zero. There should be some message about this to be exported.
Change History
comment:2 Changed 7 years ago by Peter Peterson
- Status changed from new to inprogress
Re #8878. Adding CropWorkspace to reduce memory usage.
Changeset: 926b34e72f8252e392f1e68d5b29bb46926d4acd
comment:3 Changed 7 years ago by Peter Peterson
Re #8878. Adding checks for data actually in the workspace.
Changeset: 61057dddceecd08d6321e998bb47582abf5b3dbc
comment:4 Changed 7 years ago by Peter Peterson
Re #8878. Don't double calculate the background.
Changeset: 9e0bb28cb36aabeec0c953472c04c5afb6456599
comment:5 Changed 7 years ago by Peter Peterson
- Description modified (diff)
The decision is to make FindPeakBackground::estimateBackground a public static method. This will allow for it to be called directly rather than by creating a subalgorithm and temporary workspace. This is what the actual bug was caused by lots of algorithm/workspace creation and destruction.
comment:6 Changed 7 years ago by Peter Peterson
- Milestone changed from Release 3.1 to Release 3.2
Other more pressing matters are taking over. This is being pushed to the next release.
comment:7 Changed 7 years ago by Wenduo Zhou
Fixed the segmentation fault. Refs #8878.
With the macro for OpenMP removed, GetDetOffsetMultiPeak did not crash on NOMAD's data but executed successfullly.
Changeset: deac45bbc76d8ebbbf465b8b50db010befd986da
comment:8 Changed 7 years ago by Wenduo Zhou
Refactored and cleaned codes. Refs #8878.
Changeset: 21f66d446833773b122ca989a55d1d0fb06a0f8a
comment:9 Changed 7 years ago by Wenduo Zhou
For tester
CalibrateRectangularDetectors does not crash on any data but only on NOMAD run 23234. So I strongly suggest that the tester is at SNS side in order to get access to the data.
- Before merging the branch to master, run the attached script to see how it crashes;
- Merge the branch to master, and rerun the script to check whether it crashes still;
- Check whether unit tests and system tests are passed as refactoring changes the structure of the codes significantly.
comment:10 Changed 7 years ago by Wenduo Zhou
Made core methods to be static. Refs #8878.
Changeset: 44ebe6190ae9e7bfa53f09a59fbbe645cbbf6492
comment:11 Changed 7 years ago by Wenduo Zhou
Added debugging information. Refs #8878.
Changeset: e72527d983de60b5ec5af6ae4969fc55339cf3f8
comment:15 Changed 7 years ago by Wenduo Zhou
- Status changed from inprogress to verify
- Resolution set to duplicate
With the bug fixing in ticket #9020 (FindPeakBackground) and #9033 (FindPeaks), the problem reported by this ticket does not exist anymore. So this ticket is closed as duplicate. Branch bugfix/8878_nomad_calibration_crash has been removed from github.
For tester
You can update your master branch. Do not merge any branch. Run the script in the ticket description.
comment:16 Changed 7 years ago by Peter Peterson
- Status changed from verify to verifying
- Tester set to Peter Peterson
comment:17 Changed 7 years ago by Peter Peterson
- Status changed from verifying to closed
It works on master as commented.
comment:18 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 9722