Ticket #11628 (closed: fixed)

Opened 5 years ago

Last modified 5 years ago

Mask calculation for bleed correction, option for whole tube

Reported by: Nick Draper Owned by: Nick Draper
Priority: critical Milestone: Release 3.4
Component: Direct Inelastic Keywords:
Cc: russell.ewings@… Blocked By:
Blocking: Tester: NickDraper

Description

From Russell Ewings:

I believe that the way the mask is calculated during "bleed" corrections on direct geometry inelastic instruments is incorrect. If a pixels is deemed to have a high rate, i.e. strong Bragg peak present, then the tube in which it lives is likely to suffer from "bleed" due to detector electronics problems. The entire tube should therefore be masked in the data reduction. At the moment it seems that only the high-rate pixels themselves are masked, and not the tube. An example run where this can be seen is Merlin run 24643, which was a multi-rep run with Ei=120, 43 and 22meV. The corresponding white beam vanadium run is 23684, and the hard mask file we use is MER23698.msk. All of the other parameters are the defaults used in our template script that Alex wrote.

Thanks, Russell

I would suggest we provide an option for high rate pixels, or whole tube masking.

Change History

comment:1 Changed 5 years ago by Alex Buts

  • Status changed from new to inprogress

Re #11628 Fixed diagnose for event workspaces.

Changeset: b2af8059258f1c745bc85615ba21cb984e9421bb

comment:2 Changed 5 years ago by Alex Buts

  • Cc russell.ewings@… added

comment:3 Changed 5 years ago by Alex Buts

Re #11628 fixing unit tests and more detailed description of the

CreatePSDBleedMask algorithm.

Changeset: fb077fe05c8c46d5709fa2c83221df6ced9f9cc4

comment:4 Changed 5 years ago by Alex Buts

Re #11628 unit tests for bleeding corrections

and various changes to fix&modify unit test

Changeset: 0ad28cb70bc4a4ccd6056502dcc957adf71ffee5

comment:5 Changed 5 years ago by Alex Buts

Re #11628 Changes to fix GUI

Changeset: d3dd0b4d24f8e9c7f203bc68e4042065e132f96a

comment:6 Changed 5 years ago by Alex Buts

Re #11628 Proper description for the algorithm

Changeset: 4d95c232d070c4592eef1a120fed70c52e9a673f

comment:7 Changed 5 years ago by Alex Buts

Re #11628 Minor changes done while fixing system test for MERLIN

also added warning to Integration test, stating that it does not work as expected for event workspaces.

Changeset: 4186e4be140285091371772536add744615c4284

comment:8 Changed 5 years ago by Alex Buts

Re #11628 changed MERLINReduction.nxs reference file

as bleeding corrections were wrong for some time.

Changeset: b4ca5b66eb4ec041b781d473541815332323b814

comment:9 Changed 5 years ago by Alex Buts

Re #11628 fixing LET system test

Changeset: 664374afbe4381fe496f234b94fed4300ac2b05f

comment:10 Changed 5 years ago by Alex Buts

This fixed as PR https://github.com/mantidproject/mantid/pull/668

To my disbelieve, this appears to be ticket not about bleeding corrections (though about them too a bit) but about diagnostics for direct inelastic in event mode. I heard a rumours that diagnostics does not properly work in event mode, and this ticket have clearly proven this.

To complete the ticket range of bugfixes and changes had to be implemented:

1) Fixed background test in event mode. Apparently, integration in event mode was working over random TOF region (often over the whole frame region). This has been fixed and warning has been added to Integration algorithm description.

2) Bleeding corrections need non-normalized workspace. Apparently ISIS reduction was testing bleeding on normalized workspace for ages and results were referenced in the system tests. I have changed reference file in system test to the correct one.

3) changes to (2) caused small re-factoring of the reduction code to run bleed test on non-normalized workspace. New unit test have been written and number of smaller changes and tests were modified to reflect this.

4)Description of bleeding correction algorithm was pretty poor. Partially because of this, its incorrect operations left unnoticed for so long. I've written much more advanced description, illustrating bleeding and how proper corrections look like using ticket's MER24643 file as the example. The diagnostics was initially suffering from issues 1) and 2) but now I believe that the reduction does proper corrections as have been discussed with Russell. All his remarks were addressed.

Despite all these changes, tester who is probably not an expert in Direct Inelastic could not do too much. If(when) system tests pass, one can be reasonably confident that previous reduction works fine. I have fixed MERLIN system test, which now calculates bleeding masks (It was apparently did not found any bleeding corrections before but now it does).

Do the code review. Look at the CreatePSDBleed algorithm description and check how it looks now. Physics looks right and will be additionally verified by Russell.

comment:11 Changed 5 years ago by Alex Buts

Re #11628 Trying to fix background removal when normalised by

monitor-2 (may break other normalizations)

Changeset: 5ca816b112c4b944e324fd95b2434a244e9aaef9

comment:12 Changed 5 years ago by Martyn Gigg

@abuts There is a crash in PythonScriptsTest_DirectEnergyConversionTest

comment:13 Changed 5 years ago by Alex Buts

Re #11628 reverting changes to _find_or_build_bkgr_ws

as different normalization options are envisaged.

Changeset: ab430a5ee19996b5578910135c0da2e6ff87690a

comment:14 Changed 5 years ago by Alex Buts

Re #11628 export_normalization method and unit test for it

Changeset: 420970427e41ac5be54af22b8cfa407ecb4d189c

comment:15 Changed 5 years ago by Alex Buts

Re #11628 Fixing CreatePSDBleedMask crashing Mantid on goodfrm

log with non-integer value.

Changeset: d9d87fe612625c8909194554bc453831d7efe54d

comment:16 Changed 5 years ago by Alex Buts

Re #11628 fixed background removal in multirep mode for non-event ws

Changeset: f828deae40ab8175eb60381e1fc2dc3d7deab849

comment:17 Changed 5 years ago by Alex Buts

After generating pull request I've realized that background removal in multirep mode does not work correctly when normalization is anything else but current.

This is hopefully fixed now. During the fixing this issue I've discovered couple of other minor issues (e.g. crash of Mantid if CreatePSDMask has wrong type of goodfrm log)

As new background removal (multirep removal, later removal) is not system tested and waits for physical evaluation, a tester can safely accept changes if existing system tests show that reduction runs as before. Qualitative tests suggest that background removal works fine.

Last edited 5 years ago by Alex Buts (previous) (diff)

comment:18 Changed 5 years ago by Alex Buts

Re #11628 Pylint warnings

what a waste of time!

Changeset: c541714f66a12ac936a78265ffd05d433d9e4835

comment:19 Changed 5 years ago by Alex Buts

comment:20 Changed 5 years ago by Alex Buts

Re #11628 Modified NormaliseByCurrent algorithm to return norm_factor

log value

Changeset: defbbcc0eaab2b4ad02b3a18146fb7509cadddf7

comment:21 Changed 5 years ago by Alex Buts

Re #11628 Using changes in NormalizeByCurrent algorithm

within the reduction script

Changeset: fb376b424b3e1fbee8b154d78842b20ff0c56972

comment:22 Changed 5 years ago by Alex Buts

Re #11628 minor comments and export_normalization for dependent

properties.

Changeset: b56f31a95f03eaeb7fb4e83bd4b6493236437268

comment:23 Changed 5 years ago by Alex Buts

Re #11628 documentation and correct good frame rate for event

workspace.

Changeset: 0a0a8c128ba083496c6dca9a45ad733c3f664d02

comment:24 Changed 5 years ago by Alex Buts

comment:25 Changed 5 years ago by Alex Buts

Re #11628 fixing unit test (test log name incorrect)

Changeset: 732748c1b2d419ccfbf0d67d6b2222c9c3d6d4ce

comment:26 Changed 5 years ago by Alex Buts

Re #11628 fixing conflicts with master

Merge branch 'master' into 11628_bleedingCorrections

Conflicts:

Code/Mantid/docs/source/algorithms/NormaliseByCurrent-v1.rst

Changeset: 277e5268ac9342a7f7e9dd7953b4cfab77f871bd

comment:27 Changed 5 years ago by Nick Draper

  • Status changed from inprogress to verify
  • Owner changed from Alex Buts to Nick Draper
  • Resolution set to fixed

comment:28 Changed 5 years ago by NickDraper

  • Status changed from verify to closed
  • Tester set to NickDraper

Merge pull request #668 from mantidproject/11628_bleedingCorrections

fixing bleeding corrections for ISIS direct inelastic

Full changeset: 98dcf72dd94895f51e638e0c9eeb25a4ae3fcdcc

comment:29 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 12466

Note: See TracTickets for help on using tickets.