Adds orsopy integration for .ort files and ORSO models
#131
Conversation
alexhroom
changed the title
.ort file reader.ort files and ORSO models
alexhroom
force-pushed
the
104-ort
branch
3 times, most recently
from
5fca75e to
b101bb9
Compare
| [ | ||
| item | ||
| for item in other | ||
| if hasattr(item, self.name_field) and getattr(item, self.name_field) not in self.get_names() |
There was a problem hiding this comment.
What happens if there are duplicate names in the second list (which does not have to be a ClassList) - my reading of this is that both would be added.
There was a problem hiding this comment.
Have you tested that this triggers the errors in extend()?
| project.parameters.union(model_info.parameters) | ||
| project.layers.extend(model_info.layers) | ||
| project.bulk_in.append(model_info.bulk_in) | ||
| project.bulk_out.append(model_info.bulk_out) |
There was a problem hiding this comment.
Why does parameters require union, but append/extend is ok for the others?
| project.resolutions.append( | ||
| Resolution( | ||
| name=f"{sample.name} Resolution", | ||
| type="data", |
There was a problem hiding this comment.
I'm not sure that this is required, given that a data resolution is simply an instruction to the contrast to use the fourth column in the datafile and does not supply any information in and of itself. If you imagine that you have ten datasets, each with four columns, there's no need at all to have ten data resolutions. I'd remove this and let the user decide what they're going to do about resolutions.
There was a problem hiding this comment.
how about if it just adds one data resolution and attaches all contrasts to it?
There was a problem hiding this comment.
Yes, that would definitely be better.
| Resolution( | ||
| name=f"{sample.name} Resolution", | ||
| type="constant", | ||
| source="Resolution Param 1", |
There was a problem hiding this comment.
Again, what happens for multiple datasets?
| resolution=f"{sample.name} Resolution", | ||
| model=model_info.model, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Hmm. I suppose it makes sense to have a fully defined contrast here, and if we have to have these default backgrounds and scalefactors to make use of them, but I'm a bit worried that this only does half of the job for the user, given that after running this routine they will always need to review the background, scalefactor and resolution.
What would you consider to be the guiding principle behind this routine? What do you imagine the user workflow to be? This should help us with how to proceed.
There was a problem hiding this comment.
Ideally I'd like the user workflow to be that it works 'out of the box' - that the project can just be thrown directly into a run. This of course is not amazingly feasible due to backgrounds and scalefactors not being defined, so I wanted to give them 'reasonable' default values. I'm tempted to change it to use some ORSOProject dataclass that users can then plug into a project once they've properly defined their backgrounds and so on.
|
|
||
| def orso_model_to_rat( | ||
| model: Union[orsopy.fileio.model_language.SampleModel, str], absorption: bool = False | ||
| ) -> ORSOSample: |
There was a problem hiding this comment.
The absoprtion parameter is not set in the call from ort_to_project, so how do we set absorption layers if required? Is this available to us in the ort file?
There was a problem hiding this comment.
The jupyter notebook indicates users will use this routine, so that's presumably the plan.
There was a problem hiding this comment.
whoops - i forgot to add it to ort_to_project!
| "source": [ | ||
| "## Reading in data and models from .ort files\n", | ||
| "\n", | ||
| "It is possible to read data from .ort format files at two different levels of depth." |
There was a problem hiding this comment.
I think this line could be phrased better, maybe: "It is possible to read both dataset and entire RAT projects from .ort format files"
|
@DrPaulSharp to address your comment about substrate/liquid: the only thing |
|
Ok, that makes sense. My concern though is that in |
This PR adds functionality to read
.ortdata and models. Fixes #104The interface is through three functions:
RATapi.utils.orso.load_ort_data, which loads a list of RATDataobjects from an .ort fileRATapi.utils.orso.ort_to_project, which creates a Project with as much of the file data as possible, including both data and contrasts if the model data for a file is definedRATapi.utils.orso.orso_model_to_rat, which turns an ORSO model description (given as either an orsopySampleModelor a string) into an ORSOSample dataclass, which gives bulk in and bulk out data as well as a list of layers in the model and the parameters required to define them.