Ticket #10225 (closed: fixed)
SNSPowderReduction is broken when PreserveEvents=False
Reported by: | Martyn Gigg | Owned by: | Wenduo Zhou |
---|---|---|---|
Priority: | blocker | Milestone: | Release 3.3 |
Component: | Framework | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Tester: | Vickie Lynch |
Description
It was broken in this commit with the addition of a logging statement that asks for the number of events (https://github.com/mantidproject/mantid/commit/342aca3e08d3ec21be2627f2d380f85b8bb72bfb#diff-1b114ee94e8df86b22e2cac0469acd8bR660)
This has caused the MPI system tests to fail as the PreserveEvents flag is force to false for an MPI build.
This script demonstrates the problem (based on the current master)
cal_file = "PG3_FERNS_d4832_2011_08_24.cal" char_file = "PG3_characterization_2012_02_23-HR-ILL.txt" # reduce a sum of runs - and drop it SNSPowderReduction(Instrument="PG3", RunNumber=[9829,9830], Extension="_event.nxs", Sum=True, # This is the difference with the next call PreserveEvents=False, VanadiumNumber=-1, CalibrationFile=cal_file, CharacterizationRunsFile=char_file, LowResRef=15000, RemovePromptPulseWidth=50, Binning=-0.0004, BinInDspace=True, FilterBadPulses=True, SaveAs="gsas", OutputDirectory=os.path.expanduser("~/"), FinalDataUnits="dSpacing")
Change History
comment:3 Changed 6 years ago by Wenduo Zhou
Fixed the bug. Refs #10225.
Changeset: d62f98e12748c5cf917a6a7ecb5a4708cd882629
comment:4 Changed 6 years ago by Wenduo Zhou
- Status changed from inprogress to verify
- Resolution set to fixed
comment:5 Changed 6 years ago by Martyn Gigg
- Status changed from verify to verifying
- Tester set to Martyn Gigg
comment:6 Changed 6 years ago by Martyn Gigg
- Status changed from verifying to reopened
- Resolution fixed deleted
The fix does cure the problem, however the method for checking whether the workspace is an EventWorkspace is a little non-standard.
Can we replace line #664 with
if isinstance(temp, IEventWorkspace):
as this is more standard Python and I don't see the need for the first self.preserveEvents is True check as surely we only need to check whether we have an event workspace?
comment:7 Changed 6 years ago by Martyn Gigg
I have just another place where the .__class__ is used where it doesn't need to be:
- Line #251 can be replaced with if isinstance(returned, list):
I think it would be good to clean this up now too.
comment:8 Changed 6 years ago by Michael Reuter
We need to get this cleaned up so that we can get the MPI build back in order.
comment:9 Changed 6 years ago by Wenduo Zhou
- Status changed from reopened to inprogress
Refs #10225. Replace with isinstance.
On branch bugfix/10225_snspdred_mpi_failure Changes to be committed:
modified: Framework/PythonInterface/plugins/algorithms/SNSPowderReduction.py
Changeset: cd3e78840d95a629898e54d5341187e139c5fd2e
comment:10 Changed 6 years ago by Wenduo Zhou
- Status changed from inprogress to verify
- Resolution set to fixed
For tester
Check the change, unit tests and system tests, especially the MPI one.
comment:11 Changed 6 years ago by Vickie Lynch
- Status changed from verify to verifying
- Tester changed from Martyn Gigg to Vickie Lynch
comment:12 Changed 6 years ago by Vickie Lynch
- Status changed from verifying to closed
Merge remote branch 'origin/bugfix/10225_snspdred_mpi_failure'
Full changeset: 4a9faf6b86246c7846dba3d33c5839741d064f6d
comment:13 Changed 6 years ago by Vickie Lynch
Merge remote branch 'origin/bugfix/10225_snspdred_mpi_failure'
Full changeset: 4a9faf6b86246c7846dba3d33c5839741d064f6d
comment:14 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 11067