Ticket #9536 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Convert To Energy: Mask detectors bug

Reported by: Samuel Jackson Owned by: Samuel Jackson
Priority: critical Milestone: Release 3.2
Component: Indirect Inelastic Keywords:
Cc: Blocked By:
Blocking: Tester: Owen Arnold

Description

Convert To Energy is masking bad detectors using the first workspace in a list of runs. This should be changed to be on a per run basis. Masking should be moved to separate reduction step and executed on a per workspace basis.

Attachments

TSC15166.raw (3.7 MB) - added by Samuel Jackson 6 years ago.
File for testing.
TSC15167.raw (3.7 MB) - added by Samuel Jackson 6 years ago.
File for testing.

Change History

comment:1 Changed 6 years ago by Owen Arnold

  • Status changed from new to assigned

comment:2 Changed 6 years ago by Samuel Jackson

  • Status changed from assigned to inprogress

Refs #9536 Create IdentifyBadDetectors step.

This moves the code to identify bad detectors to a separate reduction step that is executed on a per run basis, rather than for an entire set of runs.

Changeset: f9def51e41b3184b96d65e8d635b077c2fbc3f59

Changed 6 years ago by Samuel Jackson

File for testing.

Changed 6 years ago by Samuel Jackson

File for testing.

comment:3 Changed 6 years ago by Samuel Jackson

To Reproduce

First try to reproduce the problem without the changes in this ticket.

  • Open Indirect Convert To Energy interface
  • Select TOSCA as the instrument
  • Select the two raw files provided as the runs to be used.
  • Leave all other options as default.
  • Rename the reduced TSC15167 in Mantid to be called something different.
  • Open just TSC15167 in as the run to be used and run convert to energy again.
  • Compare the workspace you renamed with the workspace that was just created. There should be a small but noticeable difference in the data.

To Test

  • Check that the system tests are passing
  • Follow the instructions above, but this time both methods should produce the same result.

comment:4 Changed 6 years ago by Samuel Jackson

Refs #9536 Fix passing mask list between reduction steps.

Changeset: 0409529ae685182e8b580385f24fefd61b1b229b

comment:5 Changed 6 years ago by Samuel Jackson

  • Status changed from inprogress to verify
  • Resolution set to fixed

comment:6 Changed 6 years ago by Samuel Jackson

  • Priority changed from major to critical

Bumping this up to critical as this seriously affects energy reduction for TOSCA.

comment:7 Changed 6 years ago by Samuel Jackson

  • Status changed from verify to reopened
  • Resolution fixed deleted

comment:8 Changed 6 years ago by Samuel Jackson

  • Status changed from reopened to inprogress

Refs #9536 Update C2E system tests to check multi file reduction.

Changeset: c62ab658a817505d8c29381400623e1c59083061

comment:9 Changed 6 years ago by Samuel Jackson

Revert "Refs #9536 Update C2E system tests to check multi file reduction."

This reverts commit c62ab658a817505d8c29381400623e1c59083061.

Changeset: 158bf2add7d8dcf0a1329524f8ce3869b6edc6a6

comment:10 Changed 6 years ago by Samuel Jackson

Revert "Revert "Refs #9536 Update C2E system tests to check multi file reduction.""

This reverts commit 158bf2add7d8dcf0a1329524f8ce3869b6edc6a6.

Changeset: 9ccd8836584c246e67efd32f789702e53d9bfba1

comment:11 Changed 6 years ago by Samuel Jackson

  • Status changed from inprogress to verify
  • Resolution set to fixed

New system tests for this appear to be passing now. Whatever was conflicting on develop appears to have been resolved elsewhere.

To Reproduce

  • Open the Convert to Energy interface
  • Change the instrument to TOSCA
  • Select both of the files attached to the ticket and run the reduction.
  • Change the name of the second one to be something different (do it doesn't get overwritten)
  • Select just the second file and run the reduction.
  • Plot both workspaces and zoom in. You will notice they're slightly different.

To Test

  • Check the system tests are passing.
  • Apply the changes
  • Check that you cannot reproduce the same issue.

Remember to merge both the main repo and the system tests

Last edited 6 years ago by Samuel Jackson (previous) (diff)

comment:12 Changed 6 years ago by Owen Arnold

  • Status changed from verify to verifying
  • Tester set to Owen Arnold

comment:13 Changed 6 years ago by Owen Arnold

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/bugfix/9536_c2e_masking_bug'

Full changeset: 546eac18d996a72a523ff178dd0ef94128e99c01

comment:14 Changed 6 years ago by Owen Arnold

Merge remote-tracking branch 'origin/bugfix/9536_c2e_masking_bug'

Full changeset: a7ce87fb470659ba42dd0fd6ca5e41c9c4dfaa59

comment:15 Changed 6 years ago by Owen Arnold

Merge remote-tracking branch 'origin/bugfix/9536_c2e_masking_bug'

Full changeset: bccb05c481511719261fc66fd136ed67c2ad1aa0

comment:16 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 10379

Note: See TracTickets for help on using tickets.