Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of "param-all" to designate files encasing all model parameters #54

Closed
arokem opened this issue Aug 10, 2022 · 2 comments · Fixed by #99
Closed

Use of "param-all" to designate files encasing all model parameters #54

arokem opened this issue Aug 10, 2022 · 2 comments · Fixed by #99

Comments

@arokem
Copy link
Collaborator

arokem commented Aug 10, 2022

In #52 (comment), @Lestropie explained that:

Remind me again why tensor components need the all modifier?

By making "_param-all" compulsory for any model where all components are encased within a single file, it makes validation far easier. This makes more sense on the premise that #46 is merged. Any file with suffix "model" would always necessitate the definition of entities "model" and "param". If such models are allowed to omit the "param" entity, then the validator (and potentially software reading from such data that attempts to be somewhat model-agnostic) would need to be able to look at other contents in the directory to make sure that there are no other files belonging to the same model.

I am starting to warm up to this, but want to make sure that this works. For validation, wouldn't the following glob pattern work even without the "all" modifier? *desc-dti_param*_model.nii.gz

@Lestropie
Copy link
Collaborator

Here's the two options, along with the requisite validation requirements for each (assuming prior merge of #46 here):

  1. _param-all is not required:

    1. Each file must satisfy something like: sub-*[_ses-*]_model-XYZ[_param-*]_model.EXT
    2. If _param-* is absent, no other file can satisfy: sub-*[_ses-*]_model-XYZ_param-*_model.EXT
  2. _param-all is required:

    1. Each file must satisfy something like: sub-*[_ses-*]_model-XYZ_param-*_model.EXT
    2. (optionally) If there is a file that satisfies sub-*[_ses-*]_model-XYZ_param-all_model.EXT, then there must not be any file that satisfies sub-*[_ses-*]_model-XYZ_param-(~all)_model.EXT
      (where "~all" means anything other that "all"; couldn't be bothered trying to figure out if glob can do such a thing)

There's maybe an argument to be made as to whether 1.ii. and 2.ii. should either both be requisite or both be optional; but this is how I've conceived of it myself and therefore why I thought 2. was the better option. Happy to consider the counter argument.

For validation, wouldn't the following glob pattern work even without the "all" modifier? *desc-dti_param*_model.nii.gz

No, that glob would necessitate that "_param-*" must appear, which is precisely the opposite of what enforcing the presence of "_param-all" for a model with a single-NIfTI output does.

@Lestropie
Copy link
Collaborator

In retrospect, I don't like "_param-all". In #92, "OrientationEncoding"["Type"] = "param" has been removed. Further, in #74, "tensor" was made an orientation encoding type, disentangling that from "the parameter encoded in the map". So even in the scenario where a pipeline writes a single NIfTI image with the six tensor components and nothing else, I would nevertheless advocate that that image should be named based on the quantitative parameter being encoded in that image being ADC / diffusivity, it's just as a tensor rather than a scalar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants