Ticket #10606 (closed: fixed)
Modify IndirectTransmission algorithm to support BASIS
Reported by: | Dan Nixon | Owned by: | Dan Nixon |
---|---|---|---|
Priority: | major | Milestone: | Release 3.3 |
Component: | Indirect Inelastic | Keywords: | |
Cc: | borreguero@… | Blocked By: | |
Blocking: | Tester: | Jose Borreguero |
Description
Currently this algorithm is tied to ISIS instruments but there is interest for this to be used with BASIS data also.
Change History
comment:2 Changed 6 years ago by Dan Nixon
- Status changed from assigned to inprogress
PyLint refactoring in IndirectTransmission
Refs #10606
Changeset: 45282e10f6aa908884780a98de754aa4230438f5
comment:4 Changed 6 years ago by Jose Borreguero
- Cc borreguero@… added
I added me to the the CC list
comment:5 Changed 6 years ago by Dan Nixon
Modified algorithm and unit tests for BASIS
Added analyser and reflection validation.
Refs #10606
Changeset: ad82d54d6145d81ea8987c8b319b6efa96102ab3
comment:6 follow-up: ↓ 9 Changed 6 years ago by Dan Nixon
To test:
- Open algorithm IndirectTransmission
- Select BASIS, silicon, 111
- Enter formula H2-O
- Provide an output workspace
- Run
- Output table should be created
- Change analyser to graphite
- Run
- Validation should fail on analyser
comment:7 Changed 6 years ago by Dan Nixon
- Status changed from inprogress to verify
- Resolution set to fixed
comment:8 Changed 6 years ago by Jose Borreguero
- Status changed from verify to verifying
- Tester set to Jose Borreguero
comment:9 in reply to: ↑ 6 Changed 6 years ago by Jose Borreguero
I've been scratching my head monkey-style until realized that the algorithm is "IndirectTransmission", not "IndirectTransmissionMonitor" hahaha
Replying to Dan Nixon:
To test:
- Open algorithm IndirectTransmissionMonitor
- Select BASIS, silicon, 111
- Enter formula H2-O
- Provide an output workspace
- Run
- Output table should be created
- Change analyser to graphite
- Run
- Validation should fail on analyser
comment:10 Changed 6 years ago by Jose Borreguero
- Status changed from verifying to reopened
- Resolution fixed deleted
comment:11 follow-up: ↓ 12 Changed 6 years ago by Jose Borreguero
Two minor things:
(1) Add units to 'NumberDensity' property in the 'doc' argument of the declareProperty method. Unfortunately, I can only guess it's mol/cc . Spencer, do you know the units?
(2) Add units to the 'Thickness' property in the 'doc' argument of the declareProperty method. I assume the unit is milimiter.
More important: I know that a property can be enabled or visible depending on another property. I now wonder whether it's possible to change the initialization list for the 'Analyser' validator based on the selected instrument. For instance, 'BASIS' has only silicon analyser, so it is confusing being able to select four different analysers if one chooses BASIS as instrument.
comment:12 in reply to: ↑ 11 Changed 6 years ago by Dan Nixon
I'll check with Spencer for the units, but I agree it is probably worth documenting them.
How do you set validators based on other properties? I agree that this would probably be a good idea but this algorithm is going to have its own UI anyway (part of #10620) which will have proper support for instrument selection (as per IDR).
comment:13 Changed 6 years ago by Jose Borreguero
If there's a planned interface containing this algorithm, then I don't think it is necessary to insert dependencies among the algorithm properties. Instead, leave that to the interface, as you suggest.
comment:14 Changed 6 years ago by Dan Nixon
I was in doubt as to if I should even include the validation step here, as it has to run the LoadEmptyInstrument and LoadParameterFile algorithms to just validate the input.
I'll probably just move this to validation at execution time and assume most users will go through the interface.
comment:15 Changed 6 years ago by Dan Nixon
- Status changed from reopened to inprogress
Merge branch 'master' into feature/10606_indirect_transmission_support_basis
Refs #10606
Changeset: cee553a526ee656c2892da9b930af6242745e791
comment:16 Changed 6 years ago by Dan Nixon
comment:17 Changed 6 years ago by Dan Nixon
comment:18 Changed 6 years ago by Dan Nixon
New testing instructions:
- Open algorithm IndirectTransmission
- Select BASIS, silicon, 111
- Enter formula H2-O
- Provide an output workspace
- Run
- Output table should be created
- Change analyser to graphite
- Run
- Algorithm will fail with exception message about analyser not matching instrument
comment:19 Changed 6 years ago by Dan Nixon
- Status changed from inprogress to verify
- Resolution set to fixed
comment:21 Changed 6 years ago by Jose Borreguero
- Status changed from verifying to reopened
- Resolution fixed deleted
comment:22 Changed 6 years ago by Jose Borreguero
Just a minor thing: can you please change?
Number denisty. Default=0.1
to
Number density (atoms/Angstrom^3). Default=0.1
and also change
Sample thickness. Default=0.1
to
Sample thickness (cm). Default=0.1
comment:23 Changed 6 years ago by Dan Nixon
- Status changed from reopened to inprogress
Added units to density and thickness props
Refs #10606
Changeset: dbf4ee97e47eebf34c8117eed7e599b874599e92
comment:24 Changed 6 years ago by Dan Nixon
- Status changed from inprogress to verify
- Resolution set to fixed
comment:25 Changed 6 years ago by Dan Nixon
Not sure how I even managed to miss that.
comment:27 Changed 6 years ago by Jose Borreguero
- Status changed from verifying to closed
Works fine
comment:28 Changed 6 years ago by Dan Nixon
Merge branch 'master' into feature/10606_indirect_transmission_support_basis
Refs #10606
Full changeset: cee553a526ee656c2892da9b930af6242745e791
comment:29 Changed 6 years ago by Harry Jeffery
Merge remote-tracking branch 'origin/feature/10606_indirect_transmission_support_basis'
Full changeset: e7bcea2ad0c0217e26f17a932172dd09bf41c4cb
comment:30 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 11448