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

Concatenating different runs #43

Closed
arokem opened this issue Dec 4, 2019 · 11 comments
Closed

Concatenating different runs #43

arokem opened this issue Dec 4, 2019 · 11 comments
Milestone

Comments

@arokem
Copy link
Collaborator

arokem commented Dec 4, 2019

Some protocol/scanner combinations require starting the scanner multiple times (as an example, some GE scanners cannot collect more than 150 (160?) volumes in a single scan, so if you want to collect HCP-like data, you're going to have to scan (at least) twice.

How to handle this? My suggestion:

  1. Within each run, register the b=0 volumes to the first b=0 volume in the run.
  2. Compute a mean b=0 for each run.
  3. Register the mean b=0 volume from runs 1...n to the mean b=0 in run 0.
  4. Move all the volumes in each run by applying the registration computed in step 3 for that run.
  5. Concatenate all the volumes from step 4 across runs and register each volume to the overall mean b=0 volume, storing the combined transformation for each volume (including that computed in step 3 + that computed in step 5).

The advantage of this algorithm is that I think that it might work well for different situations (e.g., a subject who moved a lot between runs, vs. a subject who stayed very still, where the main difference would be the magnitude of the transformation computed in step 3).

Risks I can think of are asymmetric propagation of error between different runs (run 0 vs other runs in particular), but I would have think more about that.

Other thoughts/suggestions?

@arokem
Copy link
Collaborator Author

arokem commented Dec 4, 2019

I should say, this goes hand in hand with a proposed change to the BIDS spec to allow multiple runs of diffusion within a session.

@arokem
Copy link
Collaborator Author

arokem commented Dec 5, 2019

Tagging in @jyeatman for his thoughts: I think that you've collected a lot of data in this format (e.g., different runs with different b-values). If I remember correctly, your approach in the past has been to separately preprocess each run and then combine after preprocessing. What are your thoughts on the procedure outlined here?

@oesteban
Copy link
Member

oesteban commented Dec 5, 2019

Looking forward to Jason's opinion. And agree with @arokem that it feels best to process each run separately.

@oesteban
Copy link
Member

oesteban commented Dec 5, 2019

I should say, this goes hand in hand with a proposed change to the BIDS spec to allow multiple runs of diffusion within a session.

Working on this right now. It seems the run- entity is allowed for DWI. I'm trying to think whether additional metadata would be appropriate and I might take a stab at bids-standard/bids-specification#239 and bids-standard/bids-specification#263 at once.

@jyeatman
Copy link

jyeatman commented Dec 5, 2019

Yes there are many times when you would want to concatenate separate runs. Ariel's proposed pipeline is good. Here are some examples of when we concatenate across separate runs:
-Many older scanner softwares can only collect 1 b-value in a run. So multi-shell data is collected across separate runs but should (usually) be concatenated for analysis
-Reversed phase encode volumes are usually collected in separate runs

A general way to solve this might be a user input that notes which volumes to concatenate and which to process separately

@oesteban
Copy link
Member

Interesting thread @dPys linked from the shared document https://community.mrtrix.org/t/organizing-data-for-dwipreproc/1846/10

@oesteban
Copy link
Member

Comments on the proposed procedure:

  1. Within each run, register the b=0 volumes to the first b=0 volume in the run.

Questions:

  • Run an initial N4 step on each to make brain-extraction and registration more reliable? Can the B1 field be assumed constant within runs? Can it across runs? (if so, you could use the field estimated on the first b=0)
  • Independently brain-extract each b=0 before registration?
  • Run regression to the mean (within the brain mask) of the first b=0 to account for signal drift?
  • Tool for registration:
  1. Compute a mean b=0 for each run.
  • Run N4 to remove B1 bias field on each average, using the brain mask for a more accurate estimation?
  • Re-calculate a more precise brainmask?

/cc @josephmje - how do you see these steps 1 & 2, in light of #25?

  1. Register the mean b=0 volume from runs 2...n to the mean b=0 in run 1.
  2. Move all the volumes in each run by applying the registration computed in step 3 for that run.
  3. Concatenate all the volumes from step 4 across runs and register each volume to the overall mean b=0 volume, storing the combined transformation for each volume (including that computed in step 3 + that computed in step 5).

I'd like to propose an alternative here:
3. Run registration within each run to the average b=0 of that run.
4. Run registration between b=0 averages.
5. Combine transforms obtained in 3 and 4, instead of resampling data ("move") before the final registration step.

If there is a basis to think an overall registration step would be interesting, the resampling and registration step would still be a refinement option. If the registration tool we finally choose can take a list of affine transforms for initialization, then this overall registration step would certainly be a good option.

@arokem
Copy link
Collaborator Author

arokem commented Jan 13, 2020 via email

@oesteban
Copy link
Member

In general, I am not sure that I understand why you would apply bias correction to DWI. In most applications I can think of, one normalizes the DWI by the b=0 and this would presumably divide out the bias term(?)

I'm not suggesting doing the bias correction on the DWI, just the b=0 - and because it potentially makes registration more robust.

Since the average b=0 will be used to normalize DWI signal (in model fitting, i.e., beyond the scope of dMRIPrep potentially), it makes sense to keep the INU corrected b=0 internal for registration processes and only expose the average b=0 that the user will use for normalization (and canceling out the INU).

One more option is to use the DIPY affine registration (see https://dipy.org/documentation/1.1.1./examples_built/affine_registration_3d/). It's a Python + Cython only implementation that should be very similar to antsMotionCorr

Is that implementation actually using ANTS underneath? If so, I'd rather stick with Nipype with the hopes that the next version of the workflow will allow us to do really cool stuff.

I need to think about this a bit more, but that sounds right to me.

Yup! food for though tomorrow :)

@oesteban oesteban added this to the 20.0.0 milestone Mar 24, 2020
@oesteban oesteban mentioned this issue Sep 15, 2020
2 tasks
oesteban added a commit to oesteban/bids-specification that referenced this issue Sep 28, 2020
This PR addresses the problem @mattcieslak spotted at in bids-standard#239.

This enhancement (WIP) basically allows for researchers to encode
the protocol's intent regarding fieldmaps.

As @satra introduced in bids-standard#239 (comment),
BIDS "*could encode intent and automation. Whether it should is a community decision."

This PR proposes a solution to encoding the intent. It doesn't modify
anything to allow also encoding automation.

The PR attempts to be backwards compatible.

I'm submitting this draft PR to open discussions and looking forward to
feedback.

Resolves: bids-standard#239.
References: #263, nipreps/dmriprep#43, bids-standard/bids-2-devel#39
@oesteban
Copy link
Member

Okay, friends, I took a stab and drafted something for the BIDS specs. Please send your feedback to bids-standard/bids-specification#622.

I will try to address the actual problem stated originally (i.e., how to encode the intent of the researcher regarding the combination of dwi runs) in another separate PR today or tomorrow.

oesteban added a commit to oesteban/bids-specification that referenced this issue Sep 28, 2020
Addresses the use case of DWI sequencies that need to be split in several
runs because, e.g., the scanner would exceed temperature specifications
due to fast gradient-switching.

This PR also refactors the text of DWIs to better accomodate the addition
and make DWIs more consistent with the rest of the file.

The approach to solve this is similar to that of bids-standard#622 for fieldmaps.

References: nipreps/dmriprep#43.
oesteban added a commit to oesteban/bids-specification that referenced this issue Oct 6, 2020
Addresses the use case of DWI sequencies that need to be split in several
runs because, e.g., the scanner would exceed temperature specifications
due to fast gradient-switching.

This PR also refactors the text of DWIs to better accomodate the addition
and make DWIs more consistent with the rest of the file.

The approach to solve this is similar to that of bids-standard#622 for fieldmaps.

References: nipreps/dmriprep#43.
oesteban added a commit to oesteban/bids-specification that referenced this issue Oct 12, 2020
Addresses the use case of DWI sequencies that need to be split in several
runs because, e.g., the scanner would exceed temperature specifications
due to fast gradient-switching.

This PR also refactors the text of DWIs to better accomodate the addition
and make DWIs more consistent with the rest of the file.

The approach to solve this is similar to that of bids-standard#622 for fieldmaps.

References: nipreps/dmriprep#43.
oesteban added a commit to oesteban/bids-specification that referenced this issue Oct 23, 2020
This PR addresses the problem @mattcieslak spotted at in bids-standard#239.

This enhancement (WIP) basically allows for researchers to encode
the protocol's intent regarding fieldmaps.

As @satra introduced in bids-standard#239 (comment),
BIDS "*could encode intent and automation. Whether it should is a community decision."

This PR proposes a solution to encoding the intent. It doesn't modify
anything to allow also encoding automation.

The PR attempts to be backwards compatible.

I'm submitting this draft PR to open discussions and looking forward to
feedback.

Resolves: bids-standard#239.
References: #263, nipreps/dmriprep#43, bids-standard/bids-2-devel#39
oesteban added a commit to oesteban/bids-specification that referenced this issue Oct 23, 2020
This PR addresses the problem @mattcieslak spotted at in bids-standard#239.

This enhancement (WIP) basically allows for researchers to encode
the protocol's intent regarding fieldmaps.

As @satra introduced in bids-standard#239 (comment),
BIDS "*could encode intent and automation. Whether it should is a community decision."

This PR proposes a solution to encoding the intent. It doesn't modify
anything to allow also encoding automation.

The PR attempts to be backwards compatible, and is based off of bids-standard#651,
where the text about fieldmaps is being revised.

I'm submitting this draft PR to open discussions and looking forward to
feedback.

Resolves: bids-standard#239.
Depends: bids-standard#651.
References: #263, nipreps/dmriprep#43, bids-standard/bids-2-devel#39
oesteban added a commit to oesteban/bids-specification that referenced this issue Oct 26, 2020
This PR addresses the problem @mattcieslak spotted at in bids-standard#239.

This enhancement (WIP) basically allows for researchers to encode
the protocol's intent regarding fieldmaps.

As @satra introduced in bids-standard#239 (comment),
BIDS "*could encode intent and automation. Whether it should is a community decision."

This PR proposes a solution to encoding the intent. It doesn't modify
anything to allow also encoding automation.

The PR attempts to be backwards compatible, and is based off of bids-standard#651,
where the text about fieldmaps is being revised.

I'm submitting this draft PR to open discussions and looking forward to
feedback.

Resolves: bids-standard#239.
Depends: bids-standard#651.
References: #263, nipreps/dmriprep#43, bids-standard/bids-2-devel#39
oesteban added a commit to oesteban/bids-specification that referenced this issue Oct 26, 2020
This PR addresses the problem @mattcieslak spotted at in bids-standard#239.

This enhancement (WIP) basically allows for researchers to encode
the protocol's intent regarding fieldmaps.

As @satra introduced in bids-standard#239 (comment),
BIDS "*could encode intent and automation. Whether it should is a community decision."

This PR proposes a solution to encoding the intent. It doesn't modify
anything to allow also encoding automation.

The PR attempts to be backwards compatible, and is based off of bids-standard#651,
where the text about fieldmaps is being revised.

I'm submitting this draft PR to open discussions and looking forward to
feedback.

Resolves: bids-standard#239.
Depends: bids-standard#651.
References: #263, nipreps/dmriprep#43, bids-standard/bids-2-devel#39
oesteban added a commit that referenced this issue Oct 27, 2020
I have taken a stab at mapping the whiteboard scribbles of our sprint into an SVG file.

I've realized of several things I will go over in our weekly meeting (in 30 min aprox):

- *dMRIPrep* SHOULD NOT concatenate runs (#43)
- SDC (fieldmaps) requires transferring almost all responsibilities over to *SDCFlows* - that includes some of the issues that @mattcieslak raised within #43.
- We need to reconsider where denoising should be inserted (and listen what @jelleveraart has to say about this). The question is would denoising help have a better estimation of head motion and/or Eddy deformations?

Hopefully, this will help us progress faster towards our roadmap.

Resolves: #5
@oesteban
Copy link
Member

We just touched sideways on the topic in our latest meeting, but I think dMRIPrep should not attempt to combine these runs at all.

I think our push in the BIDS spec has been very positive. With those changes and dMRIPrep's outputs, it will be possible for researchers to combine data the right way.

Closing the issue (feel free to reopen if you don't agree with this take)

oesteban added a commit to oesteban/bids-specification that referenced this issue Oct 28, 2020
This PR addresses the problem @mattcieslak spotted at in bids-standard#239.

This enhancement (WIP) basically allows for researchers to encode
the protocol's intent regarding fieldmaps.

As @satra introduced in bids-standard#239 (comment),
BIDS "*could encode intent and automation. Whether it should is a community decision."

This PR proposes a solution to encoding the intent. It doesn't modify
anything to allow also encoding automation.

The PR attempts to be backwards compatible, and is based off of bids-standard#651,
where the text about fieldmaps is being revised.

I'm submitting this draft PR to open discussions and looking forward to
feedback.

Resolves: bids-standard#239.
Depends: bids-standard#651.
References: #263, nipreps/dmriprep#43, bids-standard/bids-2-devel#39
oesteban added a commit to oesteban/bids-specification that referenced this issue Oct 28, 2020
This PR addresses the problem @mattcieslak spotted at in bids-standard#239.

This enhancement (WIP) basically allows for researchers to encode
the protocol's intent regarding fieldmaps.

As @satra introduced in bids-standard#239 (comment),
BIDS "*could encode intent and automation. Whether it should is a community decision."

This PR proposes a solution to encoding the intent. It doesn't modify
anything to allow also encoding automation.

The PR attempts to be backwards compatible, and is based off of bids-standard#651,
where the text about fieldmaps is being revised.

I'm submitting this draft PR to open discussions and looking forward to
feedback.

Resolves: bids-standard#239.
Depends: bids-standard#651.
References: #263, nipreps/dmriprep#43, bids-standard/bids-2-devel#39
oesteban added a commit to oesteban/bids-specification that referenced this issue Nov 6, 2020
This PR addresses the problem @mattcieslak spotted at in bids-standard#239.

This enhancement (WIP) basically allows for researchers to encode
the protocol's intent regarding fieldmaps.

As @satra introduced in bids-standard#239 (comment),
BIDS "*could encode intent and automation. Whether it should is a community decision."

This PR proposes a solution to encoding the intent. It doesn't modify
anything to allow also encoding automation.

The PR attempts to be backwards compatible, and is based off of bids-standard#651,
where the text about fieldmaps is being revised.

I'm submitting this draft PR to open discussions and looking forward to
feedback.

Resolves: bids-standard#239.
Depends: bids-standard#651.
References: #263, nipreps/dmriprep#43, bids-standard/bids-2-devel#39
oesteban added a commit to oesteban/bids-specification that referenced this issue Apr 5, 2021
This PR addresses the problem @mattcieslak spotted at in bids-standard#239.

This enhancement (WIP) basically allows for researchers to encode
the protocol's intent regarding fieldmaps.

As @satra introduced in bids-standard#239 (comment),
BIDS "*could encode intent and automation. Whether it should is a community decision."

This PR proposes a solution to encoding the intent. It doesn't modify
anything to allow also encoding automation.

The PR attempts to be backwards compatible, and is based off of bids-standard#651,
where the text about fieldmaps is being revised.

I'm submitting this draft PR to open discussions and looking forward to
feedback.

Resolves: bids-standard#239.
Depends: bids-standard#651.
References: #263, nipreps/dmriprep#43, bids-standard/bids-2-devel#39
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

No branches or pull requests

3 participants