[ENH] Add res and den keywords to indicate resolution of resampled data
#301
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ Template: | |
| <pipeline_name>/ | ||
| sub-<participant_label>/ | ||
| anat|func|dwi/ | ||
| <source_keywords>[_space-<space>][_res-<index>][_den-<label>][_desc-<label>]_<suffix>.<ext> | ||
oesteban marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ``` | ||
|
|
||
| Processing in this context means transformations of data that does not change | ||
|
|
@@ -30,13 +30,22 @@ Note that even though `space` and `desc` are | |
| optional at least one of them needs to be defined to avoid name conflict with | ||
| the raw file. | ||
|
|
||
| **Sampling**. When two or more instances of a given derivative are provided with resolution | ||
| (or surface sampling density) being the only difference between them, then the `res` | ||
| (for *resolution* of regularly sampled N-D data) and/or `den` (for *density* of non-parametric | ||
| surfaces) SHOULD be used to avoid name conflicts. | ||
| Note that only files combining both regularly sampled (e.g., gridded) and surface sampled data | ||
| (and their downstream derivatives) are allowed to present both `res` and `den` keywords | ||
| simultaneously. | ||
|
|
||
| Examples: | ||
|
|
||
| ```Text | ||
| pipeline1/ | ||
| sub-001/ | ||
| func/ | ||
| sub-001_task-rest_run-1_space-MNI305_res-1_bold.nii.gz | ||
| sub-001_task-rest_run-1_space-MNI305_res-2_bold.nii.gz | ||
| sub-001_task-rest_run-1_space-MNI305_bold.json | ||
| ``` | ||
|
|
||
effigies marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
@@ -62,9 +71,50 @@ makes them invalid (e.g., if a source 4D image is averaged to create a single | |
| static volume, a SamplingFrequency property would no longer be relevant). In | ||
| addition, all processed files include the following metadata JSON fields: | ||
|
|
||
| | **Key name** | **Description** | | ||
| | ------------- | ---------------------------------------------------------------------------------------------------------------------------- | | ||
| | SkullStripped | REQUIRED. Boolean. Whether the volume was skull stripped (non-brain voxels set to zero) or not. | | ||
| | Resolution | REQUIRED if `res` keyword present. Dictionary of Indexes to Strings. Specifies the interpretation of the resolution keyword. | | ||
| | Density | REQUIRED if `den` keyword present. Dictionary of Strings to Strings. Specifies the interpretation of the resolution keyword. | | ||
|
There was a problem hiding this comment. Our usual way to disambiguate entities is something like:
The switch to a dictionary indexed by strings doesn't appear to be justified anywhere in the proposed text. And for resolution, if it's an index, should it not be an integer, rather than a string of digits? There was a problem hiding this comment. I'm very happy to elaborate more on this:
oesteban marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| Example JSON file corresponding to `pipeline1/sub-001/func/sub-001_task-rest_run-1_space-MNI305_bold.json` above: | ||
|
|
||
| ```JSON | ||
| { | ||
| "SkullStripped": true, | ||
| "Resolution": { | ||
| "1": "Matched with original BOLD resolution (2x2x3 mm^3)", | ||
| "2": "Matched with high-resolution T1w (0.7mm, isotropic)" | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| Example of CIFTI files (i.e., a file that combines regularly sampled data | ||
| and non-parametric surfaces) having both `res` and `den` keywords: | ||
|
|
||
| ```Text | ||
| pipeline1/ | ||
| sub-001/ | ||
| func/ | ||
| sub-001_task-rest_run-1_res-1_den-10k_bold.dtseries.nii | ||
| sub-001_task-rest_run-1_res-1_den-41k_bold.dtseries.nii | ||
| sub-001_task-rest_run-1_bold.dtseries.json | ||
| ``` | ||
|
|
||
| And the corresponding `sub-001_task-rest_run-1_bold.dtseries.json` file: | ||
|
|
||
| ```JSON | ||
| { | ||
| "SkullStripped": true, | ||
| "Resolution": { | ||
| "1": "Matched with MNI152NLin6Asym 1.6mm isotropic" | ||
| }, | ||
| "Density": { | ||
| "10k": "10242 vertices per hemisphere (5th order icosahedron)", | ||
| "41k": "40962 vertices per hemisphere (6th order icosahedron)" | ||
|
There was a problem hiding this comment. just like these say btw, this may be the first use of a dictionary as a value in bids. i know this was discussed a long time back in the context of pybids, and in our derivatives meeting to stay away from, so may be worth a little ping to bep leads and others. There was a problem hiding this comment.
Sure
I just added a suggestion from @effigies that would provide an alternative to dicts. However, it seems useful to have all resolutions in just one place. |
||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ## Masks | ||
|
|
||
|
|
@@ -84,7 +134,7 @@ be set to `orig`. | |
| JSON metadata fields: | ||
|
|
||
| | **Key name** | **Description** | | ||
| | ------------ |----------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | RawSources | Same as defined in [Introduction](01-introduction.md), but elevated from OPTIONAL to REQUIRED | | ||
| | Type | RECOMMENDED. Short identifier of the mask. Reserved values: `Brain` - brain mask, `Lesion` - lesion mask, `Face` - face mask, `ROI` - ROI mask | | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indices thing feels weird, as opposed to a (potentially) descriptive label, given that the indices don't mean anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned below, indices allow for the developer to impose some explicit order they want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the index made explicit, i.e. where is the lookup-table that allows me to see what index value "1" signifies? Is the lookup table a TSV or a JSON?
Is the label in
den-labelfree to choose by the researcher, or is it limited to a controlled vocabulary?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the index look-up is in the sidecar, specifically here.
On my reading, it seems that
denis free text (rather than a controlled vocabulary). I'm not sure what the controlled vocabulary would be, here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @emdupre, that clarifies.
I think it would be better if both
resanddenwould allow strings rather than digits. This allows them to carry explicit meaning (like "1mm", "2mm"). Digits to indicate indices suffer from some ambiguities, i.e. should they start Python/C style 0-offset or MATLAB/Fortran style 1-offset? And is it allowed to use them in non-successive order like 1, 3, 5, 7? The developer can (and probably should) always start by reading the JSON, and subsequently loop over the keys of the dictionary.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After thinking a bit more about this (and similar ideas from @robertoostenveld above) - would you agree in relaxing the value of
resto be a<label>(and hence, consistent withden-, which is nice), but including the recommendation to use one-based, consecutive indexes?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm still missing what the 1-based consecutive index recommendation really helps with, given that labels can be arbitrary. and how would you differentiate across datasets that follow the recommendation and those that don't?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
res-333mmfor a NIfTI with 2x2x2 or that has an affine with shearing)The recommendation does not allow you to remove the metadata in the JSON. Other than that, why would you need to know whether the dataset sticks with the recommendation or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Umm - I'm a little confused - I also believe that it should be a
res-<label>and notres-<index>.I simple alphabetical sort of labels is probably all the ordering we would need?
I really hate the idea of res "2x2x2mm" being associated with index 3...that seems way too confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I modified the PR and now it proposes
res-<label>. However, I left a recommendation indicating the use of indexes. The question is whether that should be removed or not. I could also expand the recommendation box in the docs to include the three reasons I gave above.