[ENH] Registration tools #78
Conversation
|
Best reviewed: commit by commit
Optimal code review plan (1 warning)
|
|
This looks good to me. I think that the travis failure is because we need to up our nibabel requirements (to 3.0.1) to comply with niworkflows requirements. |
|
Great, I will proceed to write some unit tests for it then (unless @oesteban has any additional revision requests). |
See travis errors in nipreps#76,nipreps#77, nipreps#78, etc.
dmriprep/utils/register.py
Outdated
| def apply_affine(moving, static, transform_affine, invert=False): | ||
| """Apply an affine to transform an image from one space to another. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| moving : array | ||
| The image to be resampled | ||
|
|
||
| static : array | ||
|
|
||
| Returns | ||
| ------- | ||
| warped_img : the moving array warped into the static array's space. | ||
|
|
||
| """ | ||
| affine_map = AffineMap( | ||
| transform_affine, static.shape, static.affine, moving.shape, moving.affine | ||
| ) | ||
| if invert is True: | ||
| warped_arr = affine_map.transform_inverse(np.asarray(moving.dataobj)) | ||
| else: | ||
| warped_arr = affine_map.transform(np.asarray(moving.dataobj)) | ||
|
|
||
| return nb.Nifti1Image(warped_arr, static.affine) |
There was a problem hiding this comment.
@arokem - this is exactly one of the responsibilities of poldracklab/nitransforms which will finally be merged into nibabel when ready.
Would you like me to go through dipy and send a PR to use it (when it is closer to be merged into nibabel)?
(please check the code coverage of nitransforms... that was one of the priorities for the project).
There was a problem hiding this comment.
Yes - nitransforms looks solid. Do I understand correctly that it ultimately uses scipy.ndimage under the hood to do the resampling? Looks like it always does spline interpolation. What happens if you want nearest neighbors? Maybe nitransforms could use dipy.align.imaffine?
There was a problem hiding this comment.
Yup, it uses scipy.ndimage. You can pass in order=0 for nn interpolation 👍 .
The idea for nitransforms is to be as lightweight as possible, as it aims to be merged in nibabel (using dipy.align.imaffine would create a circular dependency, I believe).
I'll have a look into dipy.align.imaffine and potentially ask for your permission to upstream code if that makes sense.
There was a problem hiding this comment.
Yup. Upstreaming code from dipy.align would make sense. There are some gems in there :-)
There was a problem hiding this comment.
BTW, at some point I will stop depending on scipy and implement interpolation on a single, lightweight package. Reason: allowing for 4d tensor-bspline interpolation (which might not make sense for diffusion, but will be useful in fmri).
On the diffusion side of things, such package would also include polar interpolation for gradients (and that is relevant to dmriprep). Surely dipy has something for this.
There was a problem hiding this comment.
On the diffusion side of things, such package would also include polar interpolation for gradients (and that is relevant to dmriprep). Surely dipy has something for this.
I'm not sure it does... but it would be great to have.
There was a problem hiding this comment.
To clarify @oesteban -- should we go ahead and drop this apply_affine function in favor of the nitransforms equivalent or should we wait to do that until the interpolation point if that has already been upstreamed?
dmriprep/utils/register.py
Outdated
| moving, static, static_affine, moving_affine, reg, starting_affine, params0=None | ||
| ): | ||
| transform = transform_centers_of_mass(static, static_affine, moving, moving_affine) | ||
| transformed = transform.transform(moving) |
There was a problem hiding this comment.
Does this mean that the registration will operate on the resampled image after initialization - @arokem ?
There was a problem hiding this comment.
I am not sure I understand this question. The transform object calculates the center of mass affine. This is the line in which this is applied.
There was a problem hiding this comment.
Yup, in the following registration levels, you could use the affine as initialization or use the transformed image (with identity initialization). I'm pretty sure it is the first option.
If it is the first option, I don't think we really need this resampling to happen, do we?
There was a problem hiding this comment.
Gotcha - yes, that is a very good point. Will also be relevant to dipy/dipy#2025
|
Warnings are found on analyzing the commit 756a1ef. 1 warning:
We recommend to address them as possible, for example, update outdated dependencies, fix the tool's configuration, configure If you are struggling with these errors or warnings, feel free to ask us via chat. 💬 |
Codecov Report
@@ Coverage Diff @@
## master #78 +/- ##
=========================================
- Coverage 52.71% 51.7% -1.02%
=========================================
Files 16 23 +7
Lines 901 1319 +418
Branches 111 166 +55
=========================================
+ Hits 475 682 +207
- Misses 425 626 +201
- Partials 1 11 +10
Continue to review full report at Codecov.
|
|
Hello @dPys, Thank you for updating! Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, Comment last updated at 2020-09-24 05:31:28 UTC |
oesteban
force-pushed
the
registration_tools
branch
from
1581d67
to
65e107f
Compare
Co-authored-by: Oscar Esteban <code@oscaresteban.es>
Co-authored-by: Oscar Esteban <code@oscaresteban.es>
Co-authored-by: Oscar Esteban <code@oscaresteban.es>
Co-authored-by: Oscar Esteban <code@oscaresteban.es>
Co-authored-by: Oscar Esteban <code@oscaresteban.es>
Co-authored-by: Oscar Esteban <code@oscaresteban.es>
Co-authored-by: Oscar Esteban <code@oscaresteban.es>
|
All of your suggested changes committed @oesteban :-) Merge? |
|
The new tests are failing (TravisCI). I just fixed the CircleCI failure, that's not a problem of this PR. |
*This new module adds a set of Dipy-based linear registration tools to dMRIprep to use for EMC. These are based on @arokem 's original API's, though have been modified to input/output NIfti1Images rather than numpy arrays, includes support for inverted transform application, and includes a new average affine utility (@oesteban -- might something like this already be available from nitransforms?).
The original API's have additionally been modified to draw the hyperparameters for 3Dtransform from several presets which are stored in coarse/precise affine/rigid templates now located in the config module.