Populating of IntendedFor doesn't work as intended

[pvelasco]

Summary

Originally raised in #482 (comment).

I messed up the workflow for the population of the "IntendedFor" field, and it doesn't behave as expected.
Because the population of "IntendedFor" needs to run after all the files are "bidsified", we decided that this feature would be run as a --command option, instead of the default behavior. So, if the --command populate-intended-for option is passed, that is what happens: instead of the regular workflow, process_extra_commands is called.
However, even if that option is not passed, convert also runs populate_intended_for if the POPULATE_INTENDED_FOR_OPTS dict is included in the heuristic file.

So, question @yarikoptic : should I fix this so that it is called only when passing the --command populate-intended-for argument, or only when present in the heuristic (regardless of the --command)? The advantage of running it when present in the heuristic is that you only need to run heudiconv once; if you control it via de --command argument, you need to call heudiconv twice: once for the initial classification and then again to populate the field. The advantage of controlling it via the --command is that you have more control: you can run heudiconv without the population of the intended if so you desire using a heuristic file that has the POPULATE_INTENDED_FOR_OPTS.
There is a third option: to have a new argument: --populate-intended-for, which will run the population of the "IntendeFor" fields at the end of heudiconv, so that the user doesn't need to call it twice.

Platform details:

Choose one:

• Local environment

• Container: nipy/heudiconv:latest, but then I download the current master (e747f05) and pip install it.

• Heudiconv version: 0.10.0, upgraded to master (current: e747f05)

[yarikoptic]

sorry for a long delay with the reply. Unfortunately I am not entirely sure I understand the issue in full, and now that we have months passed for me to reply - probably you @pvelasco probably neither ;)

Anyways - I think it behaves "as designed", that if heuristic has POPULATE_INTENDED_FOR_OPTS we populate IntendedFor for newly converted data. We have added --command mode of operation so others, who might have already converted the data, could just use the --command form to populate IntendedFor. And I believe I use both forms of invocation this way. I do not see why user with a heuristic which defines POPULATE_INTENDED_FOR_OPTS would need to call it twice.

Sure thing we can also make it that by default (if heuristic does not define POPULATE_INTENDED_FOR_OPTS) we could make it not None at https://github.com/nipy/heudiconv/blob/HEAD/heudiconv/convert.py#L211 but e.g. what I have in ReproIn - both Shim and Geometry. But IIRC we had an issue that in some cases there is no Shim information so then it would fail probably...

anyways -- may be you recall more details about the issue so we both could understand and fix it (if there is anything to fix)?

[neurorepro]

Hi @pvelasco, @yarikoptic, i'm joining the conversation as I'm facing a related issue.

I'd like to use and contribute to reproin to help have standardized sequence names across scanners when possible. In my situation the sequence names were specifically chosen so that to use reproin (i.e. running heudiconv only once without any manual input). I have three runs of a functional task acquired with a Siemens Prisma for which the second run does not have the exact same image geometry neither shimming somehow. The difference is minuscule but it messes up the intendedfor code we worked on.

Some thoughts coming from this situation:

• should we consider the introduction of a tolerance for shimming or image geometry differences as per this discussion ? In my case the operator may have introduced a micro-change, and so not sure if the difference in image geometry should be allowed (json avail if interested)
• this kind of situation could be solved by using the CustomAcquisitionLabel (cf here and there) we worked on previously

Now the question is: how to indicate in heudiconv the use of e.g. CustomAcquisitionLabel when using reproin as per the subject of the current issue ?

Tackling the current project where there are several field maps for several sets of tasks (cf exampe at bottom of this post), i realize that:

• the CustomAcquisitionLabel -- which is currently looking for the acq tag in fieldmaps and associating it with either the task name in case of a func or the acq name in case of something else than a func -- could be made more general by always matching to the acq tag (even if a func because a single fieldmap could be used for several tasks)
• an even better (super-seeding) solution would be a clear and explicit association between fieldmaps and the volumes they are aimed to reduce the distortions of. This is the perfect example of the usefulness of reproin in this case (cf suggestion below).

Here is a suggestion on how to explicitly indicate the association fmap <-> vol_to_apply when naming sequences at scanner level assuming a 1-to-many mapping (i can change the suggestion if assumption should be relaxed to the possibility of having several fieldmaps for the same volume):

• have a tag fid to set the identity of fieldmap: fmap_fid-<fmap-id>_...
• have a tag afid combined with the fid of the fieldmap to show the volume's associated fieldmap id: func_task-<task-name>_afid-<fmap-id>_...

This would be useful in many situations, including the example below:

• func_task-task1_run-01
• func_task-task1_run-02
• func_task-task1_run-03
• fmap FOR task1, task2 and task3
• anat-T1w
• func_task-task2_run-01
• func_task-task2_run-02
• func_task-task3
• dwi
• fmap FOR dwi
• func_task-task4
• fmap FOR task4 and task5
• func_task-task5

In that case the suggestion would be implemented as:

• func_task-task1_afid-123_run-01
• func_task-task1_afid-123_run-02
• func_task-task1_afid-123_run-03
• fmap_fid-123
• anat-T1w
• func_task-task2_afid-123_run-01
• func_task-task2_afid-123_run-02
• func_task-task3_afid-123
• dwi_afid-top
• fmap_fid-top
• func_task-task4_afid-45
• fmap_id-45
• func_task-task5_afid-45

[neurorepro]

For example when intendedFor misses a run due to slight differences in Shimming and Image Geometry, here is an example for a func sequence with 3 runs. Run 2 is different from the other two runs in several aspects (except for slice timing for which run 3 is somehow slightly different):

Difference (RUN1 vs RUN2 vs RUN3):


Image orientation
  RUN1
  "ImageOrientationPatientDICOM": [1, 0, 0, 0, 0.987414, -0.158158],
  "ImageOrientationText": "Tra>Cor(-9.1)",
  RUN2
  ### Different for RUN2 (compared to RUN 1 and 3)
  "ImageOrientationPatientDICOM": [0.99941, -0.00542864, -0.0339265, -1.85412e-08, 0.987439, -0.158002],
  "ImageOrientationText": "Tra>Cor(-9.1)>Sag(-2.0)",
  RUN3
  "ImageOrientationPatientDICOM": [1, 0, 0, 0, 0.987414, -0.158158],
  "ImageOrientationText": "Tra>Cor(-9.1)",

  RUN1
  "dcmmeta_affine": [
    [-2.0, 0.0, 0.0, 112.0],
    [0.0, 1.9748276472091675, -0.3163161277770996, -83.08629608154297],
    [0.0, 0.3163161277770996, 1.9748276472091675, -69.95376586914062],
    [0.0, 0.0, 0.0, 1.0]
],
  RUN2
### Different for RUN2 (compared to RUN 1 and 3)
  "dcmmeta_affine": [
    [-1.9988192319869995, -3.708231233190418e-08, -0.06871619075536728, 114.41260528564453],
    [0.010857278481125832, 1.9748774766921997, -0.3158179819583893, -83.7149887084961],
    [-0.06785303354263306, 0.31600457429885864, 1.9737114906311035, -66.09502410888672],
    [0.0, 0.0, 0.0, 1.0]
],
  RUN3
  "dcmmeta_affine": [
    [-2.0, 0.0, 0.0, 112.0],
    [0.0, 1.9748276472091675, -0.3163161277770996, -83.08629608154297],
    [0.0, 0.3163161277770996, 1.9748276472091675, -69.95376586914062],
    [0.0, 0.0, 0.0, 1.0]
],

  RUN1
  "ImageOrientationPatient": [1.0, 0.0, 0.0, 0.0, 0.98741380685208, -0.1581580666229],
  "ImagePositionPatient": [-1008.0, -1020.8423397953, 106.86694993027],
  RUN2
  ### Different for RUN2 (compared to RUN 1 and 3)
  "ImageOrientationPatient": [0.99940958635725, -0.0054286391578, -0.0339265172709, -1.8541157e-08, 0.98743874677083, -0.1580022828178],
  "ImagePositionPatient": [-1009.8835740835, -1015.3774704411, 140.94968886346],
  RUN3
  "ImageOrientationPatient": [1.0, 0.0, 0.0, 0.0, 0.98741380685208, -0.1581580666229],
  "ImagePositionPatient": [-1008.0, -1020.8423397953, 106.86694993027],

  RUN1
  "SpacingBetweenSlices": 2.0000000045657,
  RUN2
  ### Different for RUN2 (compared to RUN 1 and 3)
  "SpacingBetweenSlices": 1.9999999745851,
  RUN3
  "SpacingBetweenSlices": 2.0000000045657,

Shimming

RUN1
"ShimSetting": [-1245, -11654, 6449, 368, 237, -53, -59, 79],
RUN2
### Different for RUN2 (compared to RUN 1 and 3)
"ShimSetting": [-1247, -11654, 6449, 365, 231, -52, -60, 77],
RUN3
"ShimSetting": [-1245, -11654, 6449, 368, 237, -53, -59, 79],

Slice Timing

RUN1
"SliceTiming": [1.485, 0, 0.99, 0.0825, 1.0725, 0.165, 1.155, 0.2475, 1.2375, 0.33, 1.32, 0.4125, 1.4025, 0.5775, 1.5675, 0.66, 1.65, 0.7425, 1.7325, 0.825, 1.815, 0.9075, 1.8975, 0.495, 1.485, 0, 0.99, 0.0825, 1.0725, 0.165, 1.155, 0.2475, 1.2375, 0.33, 1.32, 0.4125, 1.4025, 0.5775, 1.5675, 0.66, 1.65, 0.7425, 1.7325, 0.825, 1.815, 0.9075, 1.8975, 0.495, 1.485, 0, 0.99, 0.0825, 1.0725, 0.165, 1.155, 0.2475, 1.2375, 0.33, 1.32, 0.4125, 1.4025, 0.5775, 1.5675, 0.66, 1.65, 0.7425, 1.7325, 0.825, 1.815, 0.9075, 1.8975, 0.495],
RUN2
"SliceTiming": [1.485, 0, 0.99, 0.0825, 1.0725, 0.165, 1.155, 0.2475, 1.2375, 0.33, 1.32, 0.4125, 1.4025, 0.5775, 1.5675, 0.66, 1.65, 0.7425, 1.7325, 0.825, 1.815, 0.9075, 1.8975, 0.495, 1.485, 0, 0.99, 0.0825, 1.0725, 0.165, 1.155, 0.2475, 1.2375, 0.33, 1.32, 0.4125, 1.4025, 0.5775, 1.5675, 0.66, 1.65, 0.7425, 1.7325, 0.825, 1.815, 0.9075, 1.8975, 0.495, 1.485, 0, 0.99, 0.0825, 1.0725, 0.165, 1.155, 0.2475, 1.2375, 0.33, 1.32, 0.4125, 1.4025, 0.5775, 1.5675, 0.66, 1.65, 0.7425, 1.7325, 0.825, 1.815, 0.9075, 1.8975, 0.495],
RUN3
### Different for RUN3 (compared to RUN 1 and 2)
"SliceTiming": [1.4825, 0, 0.99, 0.0825, 1.0725, 0.165, 1.1525, 0.2475, 1.235, 0.33, 1.3175, 0.4125, 1.4, 0.5775, 1.565, 0.66, 1.6475, 0.7425, 1.73, 0.825, 1.8125, 0.9075, 1.895, 0.495, 1.4825, 0, 0.99, 0.0825, 1.0725, 0.165, 1.1525, 0.2475, 1.235, 0.33, 1.3175, 0.4125, 1.4, 0.5775, 1.565, 0.66, 1.6475, 0.7425, 1.73, 0.825, 1.8125, 0.9075, 1.895, 0.495, 1.4825, 0, 0.99, 0.0825, 1.0725, 0.165, 1.1525, 0.2475, 1.235, 0.33, 1.3175, 0.4125, 1.4, 0.5775, 1.565, 0.66, 1.6475, 0.7425, 1.73, 0.825, 1.8125, 0.9075, 1.895, 0.495],

[neurorepro]

@yarikoptic i'll start working on the PR with the <fid> and <afid> tags as mentioned in the suggestion in the comment above except if you think that's not a good idea

[mirestrepo]

We are also struggling with this case! In case it helps knowing there are more people interested on this! 🙂

[mirestrepo]

Also, is leveraging B0FieldIdentifier a doos course of action here?

[yarikoptic]

@yarikoptic i'll start working on the PR with the <fid> and <afid> tags as mentioned in the suggestion in the comment above except if you think that's not a good idea

sorry for the delay... have you had any luck implementing what you have envisioned?

Some notes from my thinking on this: I am not sure if adding _fid-/_afid- it would end up being "long term safe"

• we are switching to use BIDS specification "schema" to automagically (instead of duplicate hardcoding) using defined in bids entities (need to get back to RF+ENH: provide bids schema driven support for manipulation of bids filenames #544).
• in RF+ENH: provide bids schema driven support for manipulation of bids filenames #544 IIRC I got stopped by needing to handle __dup's, not sure if similar problem would come due to ad-hoc _fid/afid_ here
• eventually BIDS might come up with fid and/or afid entities for use for something else, and then we would "conflict" here.

may be some if not of above concerns could be worked out by using those _fid/_afid after all BIDS entities, i.e. after __ in reproin, but those are then completely ignored by reproin heuristic and not yet sure how to orchestrate generic heudiconv logic to react to those specific entities.

[yarikoptic]

Also, is leveraging B0FieldIdentifier a doos course of action here?

that is interesting!

• we could have relied on populating it within heuristics (if we allow heuristics to provide somehow metadata to add to sidecards)
• likely we should populate it while figuring out IntendedFor but then it is somewhat of a chicken-egg issue -- not clear either we should rely on heuristics to provide that information or heudiconv to fill out that information (so BIDS tools could use it) ;)

edit 1: FWIW -- seems not yet used in any openneuro dataset ? (assuming that .json go to git) https://github.com/search?q=org%3AOpenNeuroDatasets+B0FieldIdentifier

[neurorepro]

Hi @yarikoptic , yes in the process of testing the fid and afid.

I understand the worry about BIDS compatibility. Maybe we could have a list of heudiconv reserved keywords and throw a warning if one day BIDS end up using one of those ?

Since the fid and afid would have a direct and useful impact very quickly (fieldmaps used across most projects, with the intended for field often read during preprocessing with tool such as fmriprep) i'm thinking it may still be worth implementing. What do you think ?

Maybe i can touch base with the BIDS group to check if that could be of interest in bids specs to ensure compatibility ?

[yarikoptic]

@neurorepro @mirestrepo - have you looked more into that B0FieldIdentifier situation? is that anything we could actually somehow deduce from DICOMS? FWIW -- there are some openneuro datasets where it does have it specified -- https://github.com/search?q=org%3AOpenNeuroDatasets+B0FieldIdentifier&type=code