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

Update fMRIPrep requirements and T2SMap interface to 0.0.9 #540

Closed
6 tasks done
tsalo opened this issue Feb 27, 2020 · 4 comments · Fixed by nipreps/fmriprep#2117
Closed
6 tasks done

Update fMRIPrep requirements and T2SMap interface to 0.0.9 #540

tsalo opened this issue Feb 27, 2020 · 4 comments · Fixed by nipreps/fmriprep#2117
Milestone

Comments

@tsalo
Copy link
Member

tsalo commented Feb 27, 2020

Summary

Changes in 0.0.9 will break fMRIPrep's T2SMap interface (particularly #416).

Next Steps

  • OPTIONAL: Change output files for t2smap_workflow and corresponding outputs in tedana_workflow to be more BIDS-like. Sort of like a light version of Update output names to match BIDS derivatives RC2 conventions #146.
    • t2sv.nii --> T2StarMap.nii.gz (given the BEP001 suffix)
    • s0v.nii --> S0Map.nii.gz (given the BEP001 suffix)
    • ts_OC.nii --> desc-optcom_bold.nii.gz (maybe)
  • Release 0.0.9
  • Change minimum tedana version in fMRIPrep to 0.0.9. Maybe just pin it to that version actually.
    • Not sure if there's anything we can do about fresh installs of old versions of fMRIPrep, except hope that people are using containers.
  • Have fMRIPrep's T2SMap interface look for nii.gz files instead of niis.
  • OPTIONAL: Remove t2star_adaptive_map and s0_adaptive_map from fmriprep.interfaces.multiecho.T2SMapOutputSpec. These aren't the adaptive maps actually- they're the "full" ones.
  • OPTIONAL: Switch to nonlinear fit in T2SMap interface. There's a minimal time cost and the results should be better.
@tsalo tsalo added this to the 0.0.9 milestone Feb 27, 2020
@tsalo tsalo mentioned this issue Feb 27, 2020
@tsalo
Copy link
Member Author

tsalo commented Apr 1, 2020

How does everyone feel about the proposed changes here?

@dowdlelt
Copy link
Collaborator

dowdlelt commented Apr 1, 2020

I have no concerns - in fact, more descriptive names can only be beneficial (other than breaking scripts). Gets my thumbs up.

@tsalo
Copy link
Member Author

tsalo commented Apr 1, 2020

On a related note, t2smap_workflow does not use --out-dir, and implementing this would alter the default output directory and thus fMRIPrep.

Also, our T2* maps will now be in seconds (#525). @emdupre do you think that will cause problems for fMRIPrep's --t2s-coreg (above and beyond its current issues)?

@emdupre
Copy link
Member

emdupre commented Apr 2, 2020

The map being in seconds won't cause any isues as far as I can see. Setting an outdir might since fMRIPrep has a series of DataSinks they set up in the background to move results around. But, if we set outdir to the current working directory in our call then behavior should proceed as expected.

As a light tangent: I think the decision was to remove --t2s-coreg as an option (see the discussion here: nipreps/fmriprep#1786 (comment) ), so I wouldn't worry about impacting that parameter in particular.

Just to be clear, though: we still create a T2StarMap and optimal combination in fMRIPrep even if --t2s-coreg isn't called (if multi-echo data is supplied); it's just that we don't use that T2StarMap in coregistration. Instead, we "just" pass around the optimal combination as our "bold" file; for example, here:

https://github.com/poldracklab/fmriprep/blob/8a10be0dd46fb1db98b217e5575908f1b3966bc1/fmriprep/workflows/bold/base.py#L696

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.

3 participants