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

[REBASED ] Symmetric Diffeomorphic registration workflow #1748

Merged
merged 22 commits into from Mar 9, 2019

Conversation

@skoudoro
Copy link
Member

commented Mar 4, 2019

The goal of this PR is to clean and rebase #1616. Look at #1616 for more information.

I use the opportunity to add more metric from the original PR. It seems that there are no tests so I will add them too on a future commit.

parichit and others added 15 commits Jul 31, 2018
Crated a new branch with clean history for the affine registration wo…
…rkflow.

1) Committing to this branch all the code from the affine_registration branch (PR 1581)
2) New PR should be opened for this branch.
1) Added the optional parameters to the Syn_registration workflow to …
…control the properties of the diffeomorphic registration.

2) Added the parameter documentation in the workflow.
3) Added the optional parameters for saving the displacement field and the option of saving the static image after applying the inverse mapping.
1) Added the command line wrapper for the diffeomorphic registration …
…workflow.

2) Updated the parameter documentation for the diffeomorphic registration workflow.
1) Fixed the PEP8 formatting issue in the dipy_align_affine and dipy_…
…apply_transform.

2) Changed the body of 'rigid()' function in align.py to call the center_of_mass function when the progressive flag is off.
1) Renamed the ApplyTransformFlow to ApplyAffineFlow to distinguish i…
…t from the Syn_Registration flow.

2) Made the changes in the command line wrapper to use the changed name for the workflow.
@codecov-io

This comment has been minimized.

Copy link

commented Mar 4, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@f276b93). Click here to learn what that means.
The diff coverage is 74.54%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1748   +/-   ##
=========================================
  Coverage          ?   83.79%           
=========================================
  Files             ?      118           
  Lines             ?    14190           
  Branches          ?     2255           
=========================================
  Hits              ?    11890           
  Misses            ?     1781           
  Partials          ?      519
Impacted Files Coverage Δ
dipy/io/image.py 100% <ø> (ø)
dipy/workflows/align.py 87.5% <74.54%> (ø)
@skoudoro skoudoro referenced this pull request Mar 5, 2019
14 of 15 tasks complete
@arokem
Copy link
Member

left a comment

Looks good! A couple of comments here.

out_warped : string, optional
Name of the warped file. If no name is given then a
suffix 'transformed' will be appended to the name of the
original input file (default 'warped_moved.nii.gz').

This comment has been minimized.

Copy link
@arokem

arokem Mar 5, 2019

Member

This is confusing: it says that if no name is given then the output will be name_of_original_file_transformed.nii.gz. but then also says that it will be warped_moved.nii.gz. That seems to be a contradiction. Please clarify.

This comment has been minimized.

Copy link
@arokem

arokem Mar 5, 2019

Member

I think it's probably just the documentation that needs clarification here?

io_it = self.get_io_iterator()
metric = metric.lower()
if metric not in ['ssd', 'cc', 'em']:
raise ValueError("Invalid similarity metric: Please"

This comment has been minimized.

Copy link
@arokem

arokem Mar 5, 2019

Member

Would be nice to say what metrics are valid.

moving_image, moving_grid2world = load_nifti(moving_file)

# Sanity check for the input image dimensions.
ImageRegistrationFlow.check_dimensions(static_image, moving_image)

This comment has been minimized.

Copy link
@arokem

arokem Mar 5, 2019

Member

Maybe make the check_dimensions a module-wide utility function, rather than a method of the ImageRegistrationFlow?

warped_moving = mapping.transform(moving_image)

# Saving the warped moving file and the alignment matrix.
save_nifti(warped_file, warped_moving, static_grid2world)

This comment has been minimized.

Copy link
@arokem

arokem Mar 5, 2019

Member

Should we also save out the transformation? That is, the X by Y by Z by 3 values of the diffeomorphism? We could then apply this transformation to other files, which may be useful.

We have an implementation of something like that here:

https://github.com/yeatmanlab/pyAFQ/blob/master/AFQ/registration.py#L138
https://github.com/yeatmanlab/pyAFQ/blob/master/AFQ/registration.py#L153

This comment has been minimized.

Copy link
@skoudoro

skoudoro Mar 5, 2019

Author Member

sounds good, will do

This comment has been minimized.

Copy link
@arokem

arokem Mar 5, 2019

Member

Great! We'll want to eventually follow up with an API to apply these transforms to input files. But that's probably for another release cycle...

This comment has been minimized.

Copy link
@skoudoro

skoudoro Mar 5, 2019

Author Member

Yes, I am thinking of changing ApplyAffineFlow to make it more generic. I want to call it ApplyTransformFlow with an option --affine or --diffeormorphic.

I will see if I have time to do that today. Otherwise, next release....

@Garyfallidis

This comment has been minimized.

Copy link
Member

commented Mar 9, 2019

Thanks good job @skoudoro and @parichit !

@Garyfallidis Garyfallidis merged commit 6b6835b into nipy:master Mar 9, 2019

4 checks passed

codecov/patch No report found to compare against
Details
codecov/project No report found to compare against
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@skoudoro skoudoro deleted the skoudoro:syn-workflow branch Mar 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.