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

ENH: b0 reference and skullstrip workflow #25

Closed
wants to merge 13 commits into from

Conversation

josephmje
Copy link
Collaborator

My previous workflow used MRtrix's dwibiascorrect but for some images, dwi2mask, which is called by dwibiascorrect would produce a mask around the cerebellum and only that region would be enhanced. This PR adapts init_bold_reference_wf from niworkflows.

However, I noticed that the mask produced by BET in the skullstrip_first_pass node includes every voxel. The output is fine but not sure if this is also a problem in niworkflows.

Copy link
Member

@oesteban oesteban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a first pass. I still need to go through the workflow.

dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
@josephmje
Copy link
Collaborator Author

I'll modify the workflow to use the new dwi.tsv file instead of bvec and bval.

Copy link
Member

@oesteban oesteban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good. I particularly like that you documented the workflows very well. Some further comments:

  • Please consider the more robust calculation of the final B0 map that I suggested.
  • We need to add some tests
  • We need to generate the reportlet so that we can refer future contributors to this PR when they ask about the reports system.

dmriprep/workflows/dwi/util.py Outdated Show resolved Hide resolved
:abbr:`INU (intensity non-uniformity)` bias field and calculates a signal
mask.

Steps of this workflow are:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking about this, and I would like to hear opinions (@arokem, @jelleveraart) about a more robust b0 computation:

Using the rough mask (I'll call it pre_mask) calculated before, what about recalculating the reference as follows:

bzeros = dwi_data[..., b0vals_mask]
bzeros = 1000 * bzeros / bzeros[pre_mask, ...].mean(axis=-1)  # Normalize to have mean intensity of 1000 within mask
b0map = np.median(bzeros, axis=-1)  # median should be more robust than average, esp. with few bzeros

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first rescaling is to deal with the average signal decay you generally see when b zeros are distributed along the scan. We could also store the average signal series (that bzeros[pre_mask, ...].mean(axis=-1)) to have an estimation of signal drop and maybe allow some exponential decay fitting down the line.

We could also robustify that calculation using median instead of mean.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it also be worth registering each b0 to the first in case there is significant motion between them? I'm not sure what opinions are on this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I would start by registering all the b0 to each other, and then registering the others to the mean of these registered b0 volumes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've temporarily gotten FSL mcflirt setup but would this be something handled by antsMultivariateTemplateConstruction?

@oesteban
Copy link
Member

oesteban commented Nov 4, 2019

@josephmje can you rebase to upstream/master?

@pep8speaks
Copy link

pep8speaks commented Nov 4, 2019

Hello @josephmje, Thank you for updating!

Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, pip install flake8 and then run flake8 dmriprep.

Comment last updated at 2019-12-16 16:38:28 UTC

@oesteban
Copy link
Member

oesteban commented Nov 4, 2019

mmm this doesn't look quite right, how have you done the rebase?

@josephmje
Copy link
Collaborator Author

mmm this doesn't look quite right, how have you done the rebase?

I used:

git fetch upstream
git rebase upstream/master

@oesteban
Copy link
Member

oesteban commented Nov 4, 2019

can you post: git remote -v?

@josephmje
Copy link
Collaborator Author

can you post: git remote -v?

origin	https://github.com/josephmje/dmriprep.git (fetch)
origin	https://github.com/josephmje/dmriprep.git (push)
upstream	https://github.com/nipreps/dmriprep.git (fetch)
upstream	https://github.com/nipreps/dmriprep.git (push)

@oesteban
Copy link
Member

oesteban commented Nov 4, 2019

Weird, did you have to force-push?

@josephmje
Copy link
Collaborator Author

Nope, I pushed normally. But I see now, it's including everything, not just my changes.

@oesteban
Copy link
Member

oesteban commented Nov 4, 2019

Can you add me as a collaborator to your fork?

@oesteban
Copy link
Member

oesteban commented Nov 4, 2019

I'm going to push to your branch. Before any kind of git-fetch or git-pull, could you follow these steps after I push?

git checkout -b enh/skullstrip-backup enh/skullstrip
git branch -D enh/skullstrip
git fetch
git checkout enh/skullstrip

@josephmje
Copy link
Collaborator Author

Thanks @oesteban . Looks much better. What was the solution (for future reference)?

@oesteban
Copy link
Member

oesteban commented Nov 4, 2019

I believe you undid the rebase at some point, and merged some other branch in.

dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/workflows/dwi/util.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
dmriprep/interfaces/images.py Outdated Show resolved Hide resolved
@codecov-io
Copy link

Codecov Report

Merging #25 into master will decrease coverage by 16.4%.
The diff coverage is 31.5%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #25       +/-   ##
===========================================
- Coverage   40.57%   24.16%   -16.41%     
===========================================
  Files           9       11        +2     
  Lines         589      662       +73     
  Branches       92       94        +2     
===========================================
- Hits          239      160       -79     
- Misses        349      501      +152     
  Partials        1        1
Impacted Files Coverage Δ
dmriprep/workflows/dwi/util.py 20.93% <20.93%> (ø)
dmriprep/interfaces/images.py 46.66% <46.66%> (ø)
dmriprep/cli/version.py 16.98% <0%> (-75.48%) ⬇️
dmriprep/cli/run.py 7.39% <0%> (-21.84%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c5d0f1...0e29500. Read the comment docs.

@pull-assistant
Copy link

pull-assistant bot commented Nov 26, 2019

Score: 0.79

Best reviewed: commit by commit


Optimal code review plan (3 warnings)

     enh: initial implementation of b0 reference and skull-stripping workfl...

     Update dmriprep/workflows/dwi/util.py

     Update dmriprep/interfaces/images.py

     Update dmriprep/interfaces/images.py

     update to use inputs according to #26

initial b0 registration adapted from niworkflows

dmriprep/workflows/dwi/util.py 73% changes removed in add mcflirt and fix ...

fix masking

dmriprep/workflows/dwi/util.py 47% changes removed in add mcflirt and fix ...

update docs and doctests

dmriprep/workflows/dwi/util.py 50% changes removed in docs: revise docstri...

     add workflow test

     add pytest fixtures

     docs: revise docstrings for napoleon parsing

     add mcflirt and fix rescale_b0 inputs and outputs

     fix imputs again

Powered by Pull Assistant. Last update fb83460 ... dd4af8d. Read the comment docs.

@oesteban
Copy link
Member

@josephmje the addition of documentation is starting to give us good feedback. The build_docs job is failing because some of the connections in the workflow are wrong:

Warning, treated as error:
/root/project/dmriprep/workflows/dwi/util.py:docstring of dmriprep.workflows.dwi.util.init_dwi_reference_wf:11:Exception occurred in plotting dmriprep-workflows-dwi-util-1
 from /root/project/docs/api/dmriprep.workflows.dwi.util.rst:
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/nipype/sphinxext/plot_workflow.py", line 485, in run_code
    exec(code, ns)
  File "<string>", line 2, in <module>
  File "/root/project/dmriprep/workflows/dwi/util.py", line 118, in init_dwi_reference_wf
    ('outputnode.skull_stripped_file', 'ref_image_brain')]),
  File "/usr/local/lib/python3.7/site-packages/nipype/pipeline/engine/workflows.py", line 214, in connect
    '\n'.join(['Some connections were not found'] + infostr))
Exception: Some connections were not found
Module rescale_b0 has no input called pre_mask

Module rescale_b0 has no output called ref_image

I've force-pushed to your branch after rebasing. I believe that:

git checkout enh/skullstrip
git pull --rebase

should work for you.
Let me know if you find any problems.

josephmje and others added 8 commits December 12, 2019 22:11
initial port from niworkflows

add initial pre-mask step

fix doc

add pre_mask

address review comments

add period

extract b0s first

fix function name typo

initial port from niworkflows

add initial pre-mask step

fix doc

add pre_mask

address review comments

add period

extract b0s first

fix function name typo

remove MatchHeader
@oesteban
Copy link
Member

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 this pull request may close these issues.

5 participants