Ticket #11063 (closed)
Parameter validation for ISIS direct reduction
Reported by: | Alex Buts | Owned by: | Alex Buts |
---|---|---|---|
Priority: | major | Milestone: | Release 3.4 |
Component: | Direct Inelastic | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | #11096, #11155, #11177 | Tester: | Dan Nixon |
Description
Current ISIS reduction does not verify its parameters until they are used.
e.g. it loads big event file and does time consuming operations on it while a mask file is missing so it fails after long work.
In addition to that, a method, which would verify user settings for Mantid&reduction script when user account created has been requested and will rely on early verifications of reduction parameters.
This ticket is to address these issues.
Change History
comment:2 Changed 6 years ago by Alex Buts
Re #11063 Better energy range tracking
This is not current ticket but something to do with enhancement of the previous one and general PropertyManager code. No point in making it separate ticket though as changes are trivial
Changeset: bb39b16e5dda038e4e70ca9ca22bb2f8b4b2ee3a
comment:3 Changed 6 years ago by Alex Buts
Re #11063 Intermediate checkout. Testing file-dependent properties
Changeset: e3286f94d0b38a881ef5a09b5e5657890d7cc845
comment:4 Changed 6 years ago by Alex Buts
Re #11063 working unit test to return file properties to check
Changeset: 5591cd198f8d6aacfe707ea2708d957fe743b0ab
comment:5 Changed 6 years ago by Alex Buts
Re #11063 working _check_file_properties method
more unit tests needed
Changeset: 9f812abcd78953bc1a8765e3869ce8a8cf1bfc50
comment:6 Changed 6 years ago by Alex Buts
Re #11063 implemented file properties validation
all seems works fine
Changeset: 806a304dbb4231f7e749417642b8d64edbdc9b12
comment:7 Changed 6 years ago by Alex Buts
Re #11063 Added validators to some property descriptor
(where it makes sense) and procedure which deploys these validators when convert_to_energy starts
Changeset: 067902548c9fbb9733b0af8c5424654c63eb1902
comment:8 Changed 6 years ago by Alex Buts
Re #11063 Exported PropertyManager validate method to ReductionWrapper
(and unit test for it)
Changeset: 6196cb4a19d791952912d568c038404442c95153
comment:9 Changed 6 years ago by Alex Buts
Re #11063 Minor comments
Changeset: 41a87af1eb7f790a84adb0fb93f85529948e5727
comment:10 Changed 6 years ago by Alex Buts
Re #11063 Simplified version of ReductionWrapper validate_result
procedure and comments to usage of this procedure (in tests)
Changeset: 53989ef8a19224c38f812d9e9d354c4878eee9f7
comment:11 Changed 6 years ago by Alex Buts
The ConvertToEnergy now validates all files necessary for reduction before running long operations and this method is exposed to ReductionWrapper to run separately. There is also further simplification to validate_result method, which would run reduction and save results if no reference file have been found.
It should also be a possibility to run these methods from separate script but I cut this ticket short as merging system tests to Mantid main would allow code reuse from system tests and it is not done yet.
comment:13 Changed 6 years ago by Martyn Gigg
Could you provide a better title for the pull request? You can edit it with the *Edit* button on the top right of the page
comment:14 Changed 6 years ago by Alex Buts
Waits for testing as pool request:
comment:15 Changed 6 years ago by Dan Nixon
Jenkins, retest this please.
comment:16 Changed 6 years ago by Dan Nixon
- Status changed from assigned to verifying
- Tester set to Dan Nixon
comment:17 Changed 6 years ago by Dan Nixon
I also seem to always get this error from the Direct Convert to Energy UI: ` UnboundLocalError: local variable 'result' referenced before assignment
at line 20 in '<Interface>' caused by line 519 in '/home/dan/testing-builds/test1/Code/Mantid/scripts/Inelastic/Direct/DirectEnergyConversion.py'
`
comment:18 Changed 6 years ago by Alex Buts
Re #11063 Minor bug in PropertiesDescriptors.py
Thanks Dan for noticing it.
Changeset: 990e006d8e18f4513cfb7d279ed27992a6157c5a
comment:19 Changed 6 years ago by Alex Buts
Re #11063 Where these conflicts on master have came from?
Merge branch 'master' into 11063_ParValidation
Conflicts:
Code/Mantid/scripts/Inelastic/Direct/DirectEnergyConversion.py Code/Mantid/scripts/Inelastic/Direct/PropertiesDescriptors.py Code/Mantid/scripts/Inelastic/Direct/PropertyManager.py Code/Mantid/scripts/Inelastic/Direct/ReductionWrapper.py Code/Mantid/scripts/Inelastic/Direct/RunDescriptor.py
Changeset: b0c3a741db8bcaa7eccc445213e79c697f713c1a
comment:20 Changed 6 years ago by Alex Buts
Re #11063 fixed typo induced by merge with master
Changeset: 3e4d5109d9bee35c18933de39ab106ef16c8b3ab
comment:21 Changed 6 years ago by Alex Buts
Re #11063 conflicts with pylint changes
Merge branch 'master' into 11063_ParValidation
Conflicts:
Code/Mantid/scripts/Inelastic/Direct/DirectEnergyConversion.py Code/Mantid/scripts/Inelastic/Direct/PropertiesDescriptors.py Code/Mantid/scripts/Inelastic/Direct/PropertyManager.py Code/Mantid/scripts/Inelastic/Direct/RunDescriptor.py
Changeset: 13a005d7ffd11a87ee6437a2276e21ffe3ca86bc
comment:23 Changed 6 years ago by Dan Nixon
There now seems to be some issues with indentation, when I try to run MariReduction.py I get the following error: ` IndentationError: unexpected indent (PropertyManager.py, line 162)
at line 3 in '/home/dan/testing-builds/test1/Code/Mantid/scripts/test/MariReduction.py' caused by line 6 in '/home/dan/testing-builds/test1/Code/Mantid/scripts/Inelastic/Direct/ReductionWrapper.py'
`
comment:25 Changed 6 years ago by Alex Buts
Re #11063 Bugs caused by pylint changes
It is amazing! These bugs are due to the changes in pylint but all are unit and system tested. How on earth Mantid Build have not identified these bugs during he build?
Changeset: be86a3ce389c463b961f9eb853c44acc954861d6
comment:26 Changed 6 years ago by Dan Nixon
Jenkins, retest this please.
comment:27 Changed 6 years ago by Alex Buts
Re #11063 More pylint conflicts
Merge branch 'master' into 11063_ParValidation
Conflicts:
Code/Mantid/scripts/Inelastic/Direct/RunDescriptor.py Code/Mantid/scripts/test/DirectEnergyConversionTest.py Code/Mantid/scripts/test/MariReduction.py Code/Mantid/scripts/test/RunDescriptorTest.py
Changeset: 1baead58179e00d1f49d9f7dd605c1d1b4733baf
comment:28 Changed 6 years ago by Alex Buts
Re #11063 One more indentation problem
Changeset: 5a252a7341b113f5b0eed5c3f16248474e9bcece
comment:29 Changed 6 years ago by Alex Buts
Re #11063 Fake commit. Just to check what Jenkins is doing
Changeset: bb9443db318eea7955173d7fb7a5907ed123a50c
comment:30 Changed 6 years ago by Martyn Gigg
Jenkins, retest this please.
comment:31 Changed 6 years ago by Dan Nixon
I'm now getting this error when trying to run MariReduction.py: ` TypeError: unbound method reduce() must be called with ReductionWrapper instance as first argument (got NoneType instance instead)
at line 120 in '/home/dan/testing-builds/test1/Code/Mantid/scripts/test/MariReduction.py' caused by line 317 in '/home/dan/testing-builds/test1/Code/Mantid/scripts/Inelastic/Direct/ReductionWrapper.py' caused by line 50 in '/home/dan/testing-builds/test1/Code/Mantid/scripts/test/MariReduction.py'
`
Other than that system tests seem to be happy.
comment:32 Changed 6 years ago by Alex Buts
Re #11063 minor typo in sample reduction
It does not run anyway, but worth making it right as somebody may decide to use it as a sample
Changeset: f21b8dd0d37c933e9b94b86fc4fa0426f76dd25d
comment:33 Changed 6 years ago by Alex Buts
Re #11063 minor comment for external code call
Changeset: a85fcc16473a2466b792c7301ebc73f7aef04b32
comment:35 Changed 6 years ago by abuts
- Status changed from verifying to closed
Merge pull request #298 from mantidproject/11063_ParValidation
Validate input and output parameters for Direct reduction
Full changeset: e575a222fb240d8e77f16eb24090d85e861c3d92
comment:36 Changed 6 years ago by Alex Buts
Merge branch '11063_ParValidation' into 11096/SumFilesAndDiagMultipleRuns
Full changeset: 36f3dc7ab83fa2f50c497435230a72d9a6c737d3
comment:37 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 11902
Re #11063 method returns all file-dependent properties needing
validation
Changeset: f5ac56e1a052a28bc3437be144a8c66b07cdf02d