Ticket #10225 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

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:1 Changed 6 years ago by Martyn Gigg

  • Status changed from new to assigned

comment:2 Changed 6 years ago by Wenduo Zhou

  • Status changed from assigned to inprogress

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?

Last edited 6 years ago by Martyn Gigg (previous) (diff)

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

Note: See TracTickets for help on using tickets.