Ticket #10684 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Hide Multirep mode inside DirectReduction

Reported by: Alex Buts Owned by: Alex Buts
Priority: major Milestone: Release 3.4
Component: Direct Inelastic Keywords:
Cc: Blocked By: #10384, #10532, #10681, #10881, #10958
Blocking: Tester: Martyn Gigg

Description

The first step to reduce variety of reduction scripts, would be to hide multi-rep mode inside modified directInelastic script

This also would allow one to remove some ugliness created by rough removal of interface from this script.

Change History

comment:1 Changed 6 years ago by Alex Buts

  • Blocked By 10681 added

comment:2 Changed 6 years ago by Nick Draper

  • Status changed from new to assigned

comment:3 Changed 6 years ago by Alex Buts

  • Milestone changed from Release 3.3 to Release 3.4

comment:4 Changed 6 years ago by Alex Buts

  • Blocked By 10836 added

comment:5 Changed 6 years ago by Alex Buts

  • Blocked By 10836 removed

comment:6 Changed 6 years ago by Alex Buts

  • Blocked By 10881 added

comment:7 Changed 6 years ago by Alex Buts

  • Blocked By 10958 added

comment:8 Changed 6 years ago by Alex Buts

  • Status changed from assigned to inprogress

Re #10684 Initial commit (attempt to construct a unit test for later

rebinning). Nothing still works

Changeset: a6f046fde28b540a1cd40ec75e5abd407304c876

comment:9 Changed 6 years ago by Alex Buts

Re #10684 Do not shift monitor workspace in any case

Changeset: c941a725370612146fbcf4ad594e0a492ae3ea87

comment:10 Changed 6 years ago by Alex Buts

Re #10684 Failing test for event mode

(this is what should be fixed)

Changeset: a78eb76256630a0b201d50f20f4895a795d0688d

comment:11 Changed 6 years ago by Alex Buts

Re #10684 Simplified Mon-2 normalization option

Changeset: e7d53862b4a07b9a43fe5577f8f8c3eeb5913243

comment:12 Changed 6 years ago by Alex Buts

Re #10684 Working later rebinning

(but more testing still needed)

Changeset: 0dc09c337cc49198755960356365c0b755634286

comment:13 Changed 6 years ago by Alex Buts

Re #10684 Minor changes to fix unit test

Changeset: cbc92525357f00f97420668633f698f3cea90c92

comment:14 Changed 6 years ago by Alex Buts

Re #10684 Forgotten energy bins in unit test +

possibility to return unbinned event workspace if requested

Changeset: 2e4e98b90ee77cf497c78a9372d22b3041dee569

comment:15 Changed 6 years ago by Alex Buts

Re #10684 Working DirectPropertyManagerTest

Changeset: 6ff71c850e379e85e715dd4e85c9918fa6a251b2

comment:16 Changed 6 years ago by Alex Buts

Re #10684 energy_bins and IncidentEnergy working in multirep_mode

though not yet unit tested and deployed in the reduction

Changeset: 2d6cff0c659966b9eec9454c0b578bcd5f1984ce

comment:17 Changed 6 years ago by Alex Buts

Re #10684 iterator over energies in multirep_mode

Changeset: 811930ea500b85ab28ee812754c8a9c56e44b70a

comment:18 Changed 6 years ago by Alex Buts

Re #10684 made DirectEnergyConversion compatible with multirep mode

Changeset: 6454e6bed79b4274603ca247590ecabd1cf3c07a

comment:19 Changed 6 years ago by Alex Buts

Re #10684 MultirepTOFSpectraList property, unit test and IDF changed

to provide information for this property.

Changeset: 8d3fb2b173cf7d65a396c00145ba2e05e087a103

comment:20 Changed 6 years ago by Alex Buts

Re #10684 find_tof_range_for_multirep function defined

Changeset: 2bce86433a211f302a0a8c65dd512884ad4e6977

comment:21 Changed 6 years ago by Alex Buts

Re #10684 method to identify tof range from ei range

and unit test for it

Changeset: e671bdd76b2c7e2cd48715b3f737f9966dcbd80c

comment:22 Changed 6 years ago by Alex Buts

Re #10684 fixing system test for MARI monitors separate

Changeset: 6fc4389d42a0b19c5d9c01474fdaabcdffe38139

comment:23 Changed 6 years ago by Alex Buts

Re #10684 fixing system test for MARI monitors separate

log analysis shows that previous workflow was actually wrong. Difference in files is miniscules (relErr~10e-3) but it worth replacing reference file to provide correct reference workflow.

Changeset: 79d91907e6621e35db39ba4c7e78f721fe5aa7e2

comment:24 Changed 6 years ago by Alex Buts

Re #10684 Kind of multirep code implemented

(not properly tested)

Changeset: d958f3ab60386493d1775b84d151a0dd2179f4c4

comment:25 Changed 6 years ago by Alex Buts

Re #10684 fixing some system tests

Changeset: 7ba8355b59b482def42d641850055a3693cad7fc

comment:26 Changed 6 years ago by Alex Buts

Re #10684 intermediate checkout to fix system tests

Changeset: d298b8e27cf56a74d1a11ba8fcee85fc62c3c90a

comment:27 Changed 6 years ago by Alex Buts

Re #10684 Trying to fix some system tests

Changeset: b372c0f4f3b6c595dd5e5eb90a11f49cccb8c092

comment:28 Changed 6 years ago by Alex Buts

Re #10684 Should fix some system tests

Changeset: 4789dc97f769ccd9a7ad01241375c5a54e1541ac

comment:29 Changed 6 years ago by Alex Buts

Re #10684 Should fix two tests

Changeset: 11cfd9306793c7c82d5d41a9c72199b93412c9b9

comment:30 Changed 6 years ago by Alex Buts

Re #10684 changed let sys test to work with new energy_bin property

this should fix this test

Changeset: 43afb8422a4eb6ca8349ca163b1c80693fb0142f

comment:31 Changed 6 years ago by Alex Buts

Re #10684 chop_ws_part method and unit test for it

Changeset: 5f8ce255f8db3b779ccfbe50fd16006a0381b71c

comment:32 Changed 6 years ago by Alex Buts

Re #10684 Unit test for multirep mode (and changes for it to work)

Changeset: 9d280b8b34b5242f71f3114964e5041f0d5f517e

comment:33 Changed 6 years ago by Alex Buts

Re #10684 Modifying EnergyBins to return abs energy range through meth

this would allow set the same range of values as you get and should also fix unit test

Changeset: a1efd817ddf51fdd99191fa44dbdc7ca9793f23c

comment:34 Changed 6 years ago by Alex Buts

Re #10684 more unit tests for multirep mode

and minor bugs find while writing them

Changeset: bb0520a8e93fa3c7cd2efce448d7a3c57cd64a60

comment:35 Changed 6 years ago by Alex Buts

Re #10684 testing and fixing repetitive execution

Changeset: 20f3c53a932136207426849b6d8b2c3d02afe999

comment:36 Changed 6 years ago by Alex Buts

Re #10684 changes to system tests to accommodate different ei-guess

Changeset: 1ba1de3a3047fe6d0f788f4e4a764863b3681340

comment:37 Changed 6 years ago by Alex Buts

Re #10684 unit test on multirep_mode abs units

and minor bugs identified during debugging this test

Changeset: a982962868158501e41fd929b1f4ee635b633c1d

comment:38 Changed 6 years ago by Martyn Gigg

The LoadInstrument documentation test is failing from the addition of the parameter to MARI_Parameters.xml - http://builds.mantidproject.org/job/develop_doctest/1153/testReport/junit/docs/algorithms_LoadInstrument-v1/exLoadInstrument/

comment:39 Changed 6 years ago by Alex Buts

Re #10684 Changes to run multirep mode abs units multiple times.

and unit test to check it. It shows that multirep does not work in multiple (autoreduction type) runs and wb runs caching is necessary for it to work (or in some way to reload workspaces)

Changeset: 8a73e5e1651d32a6fd284cfb4dc103407ef03533

comment:40 Changed 6 years ago by Alex Buts

Re #10684 Introduced MonoCorrectionFactor property

and changed unit tests to working state (failing part disabled) to be able to make intermediate commit and fix failing doctest

Changeset: 8c5b07e7a45c5a2468f8c735fb5c146a3723b14c

comment:41 Changed 6 years ago by Alex Buts

Re #10684 fixing failing doctest

Changeset: dacfc0a8e87cd2a26d4bdedce2b17765055e6ff7

comment:42 Changed 6 years ago by Alex Buts

Re #10684 MonoCorrectionFactor property with cash option and unit test

for this property

Changeset: 19a1e4933589b23a45938b2d4a0149ed30bf35ce

comment:43 Changed 6 years ago by Alex Buts

Re #10684 Multirep mode works for absolute units. (Unit test passes)

No masks yet

Changeset: 37dd5c5118be7c2f35cf6b42ca807c2798d603bd

comment:44 Changed 6 years ago by Alex Buts

Re #10684 Changes necessary for correct masking in multirep mode

Very rough changes, but simple logic. Will be test for correct logic later.

Changeset: ddc4450f57c6cf59995ecfc1ed15a1a2e81cafa8

comment:45 Changed 6 years ago by Alex Buts

Re #10684 Incorrect mon2 fixed

Changeset: de1697b572c436ae869174772934d8a0ce616c5e

comment:46 Changed 6 years ago by Alex Buts

Re #10684 Added one more system test for LET multirep mode

As changes are so big, I am leaving old system test for the time being and adding new system test. It will become the only test after discussion with instrument scientist.

Changeset: f1d11cfe2e511b83213587d13a9d71bd35957c6e

comment:47 Changed 6 years ago by Alex Buts

Re #10684 modified reduction wrapper to validate only first ws

in multirep mode

Changeset: 41e03c3bbf8e9918d0af1158886de0c3b883bad9

comment:48 Changed 6 years ago by Alex Buts

Re #10684 Accommodate bug in SaveNXSPE

Changeset: f129b378211897f9820b15721fa2e1eac3fd7c2b

comment:49 Changed 6 years ago by Alex Buts

comment:50 Changed 6 years ago by Alex Buts

Re #10684 Bug in chunking

Changeset: bf50459bd4b12c72c504dc22a3823612101c429c

comment:51 Changed 6 years ago by Alex Buts

Re #10684 minor bug in mulitifile run

Changeset: 8f17850a9916039b5c080effd5f90eb6a1a660ad

comment:52 Changed 6 years ago by Martyn Gigg

The DirectEnergyConversionTest.py is failing on develop.

That test seems very big for a unit test, should it not be a system test?

comment:53 Changed 6 years ago by Alex Buts

Re #10684 fixing unit test

Changeset: 72345314009b89b44c7c0aff9ac593831fd5440b

comment:54 Changed 6 years ago by Alex Buts

Re #10684 enable file clean_up before and after the test

This should fix recent failure, caused probably files left-overs

Changeset: 69893af6662f56b6b4218dde1b4f2f10bb9b2418

comment:55 Changed 6 years ago by Alex Buts

Re #10684 Introduce more efficient bkgd removal procedure and stop

removing background from vanadium

Changeset: 07e74af419275304c56e6a33bd91766dc10a9534

comment:56 Changed 6 years ago by Alex Buts

Re #10684 Changes in reference results due to not removing bkg

from vanadium any more.

Changeset: 66758e5c89fb20c07c9e4defbe620602a8d06f8b

comment:57 Changed 6 years ago by Alex Buts

Re #10684 Commented test shows that enumerate does not work as request

Changeset: 76ce1caa9898d2c6048634ca62a3589f4fa1c898

comment:58 Changed 6 years ago by Alex Buts

Re #10684 fix to system test with description for failure reason

Changeset: 70ea6abd31c273109fed18a9f314c17df5526689

comment:59 Changed 6 years ago by Alex Buts

Re #10684 Fixing system tests affected by not removing bkg from

vanadium

Changeset: 284f8b2a0f4d28818a5f95c6d67f2ba6dbc40a84

comment:60 Changed 6 years ago by Alex Buts

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

The changes are both in System Tests and Mantid branches (this is probably last ticket on separate System test branch)

I took various parts of user code processing multirep mode, unit & system tested it and brought inside Mantid, with aim for code working both from Web services and on user's computers.

Number of new unit tests are written and number of existing modified to validate changes. I think tester can rely on these tests passing to approve the changes. I've spoken with all instrument scientists concerned discussing the essence of changes. They agree in general but all want to try code for themselves to be completely sure, so a tester not need to worry about the this.

Code review is welcomed.

comment:61 Changed 6 years ago by Martyn Gigg

  • Status changed from verify to verifying
  • Tester set to Martyn Gigg

comment:62 Changed 6 years ago by Alex Buts

Re #10684 Minor comments and changes for the future

Changeset: 506c3f010fa786685d34794a686dbc217240c586

comment:63 Changed 6 years ago by Martyn Gigg

The last commit to systemtests here has broken the ISIS_ReductionWrapperValidate test

comment:64 Changed 6 years ago by Alex Buts

Re #10684 Decreased the accuracy of mari reduction wrapper test

to ignore changes in background removal. ReductionWrapper test verifies internal validation and this validation is not intended to be fiddled with in a way, other tests currently are. By its nature, it should not, so let's do it for the time being until decided is it better to remove or left background for vanadium and modified references files accordingly

Changeset: feb7ed09205b27d40f72aa31982f24a1b65037ca

comment:65 Changed 6 years ago by Martyn Gigg

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/feature/10684_multirepMode'

Full changeset: 510b76625d3565ac8c9ab441824e151aad4b495b

comment:66 Changed 6 years ago by Martyn Gigg

Merge remote-tracking branch 'origin/feature/10684_multirepMode'

Full changeset: 4b48bb0156ab798f1d0734165a1a0f4a663a0f6b

comment:67 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 11526

Note: See TracTickets for help on using tickets.