orso test using project workflow #412
Conversation
…ng to decompose it into pieces to apply to other tests)
DrPaulSharp
left a comment
DrPaulSharp
left a comment
There was a problem hiding this comment.
Looks good. There are a few small jobs, and I strongly recommend using complex sld values in the internal test as described, and in line with the original test.
tests/testORSOValidation.m
Outdated
| % set up test data (used as reference data) | ||
| problem.addData([proj_name,' Data'], orso_info.Data); | ||
| % define theoretical contrast | ||
| problem.addContrast('name', [proj_name,' Data'],... |
There was a problem hiding this comment.
Is this correct? How does the contract name come from the data? Note that the name is a fixed "ORSO Contrast" for each of the python tests.
There was a problem hiding this comment.
Right, so the name for the contrast is the same as that of the dataset. I think it'd be better to change to something else to remove any ambiguity.
There was a problem hiding this comment.
changed project name to ORSO contrast
| else | ||
| controls.display = 'off'; | ||
| end | ||
| [out_proj,results] = RAT(problem,controls); |
There was a problem hiding this comment.
The value of controls.display shouldn't make a difference for plotting - it's just for output to the console. This can be set to a preferred value in all circumstances - I'd recommend using "off"
There was a problem hiding this comment.
The idea was if you want to see plots you probably want to see a print-out. But I indeed have not used printouts for anything useful.
| bulk_in = orso_info.BulkInSLD; | ||
| sld = orso_info.SLD_real; | ||
| bulk_out = orso_info.BulkOutSLD; | ||
| end |
There was a problem hiding this comment.
The abelesSingle routine is designed to always have complex input for the sld array. Whilst I think using absorption as true or false accordingly is a good idea for the workflow test, I would rewrite this one to always use the complex sld values. This requires filling in empty SLD_img values with zeros, and regenerating the .mat file. The use_absorption check can reduce to use_absorption = any(orso_info.SLD_img>0);
There was a problem hiding this comment.
I didi it a bit differently but made input for abeles always complex.
…t" and couple of small typos
DrPaulSharp
left a comment
DrPaulSharp
left a comment
There was a problem hiding this comment.
One quick job and we're there!
|
|
||
| % run model simulation | ||
| controls = controlsClass(); | ||
| controls.display = 'off'; |
There was a problem hiding this comment.
Could you remove the tab here please.
What is the current behaviour (You can also link to an open issue here)?
Solves Re Make ORSO test use the full project stack #398. Current orso validation test validates
abelsSinglefunction only.What kind of change does this PR introduce, does this PR introduce a breaking change?
The test re-implements orso valudation tests using full
RATworkflow which incudes project creation, definition of project parameters and setting all appropriate properties.Other information: