-
Notifications
You must be signed in to change notification settings - Fork 429
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
[WIP] Registration public API for multi-slice 2D data #2538
base: master
Are you sure you want to change the base?
Conversation
Hello @samcoveney, Thank you for updating ! Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated at 2022-03-29 08:25:43 UTC |
Codecov Report
@@ Coverage Diff @@
## master #2538 +/- ##
========================================
Coverage 85.08% 85.08%
========================================
Files 129 127 -2
Lines 17579 17439 -140
Branches 2994 2969 -25
========================================
- Hits 14957 14838 -119
- Misses 1926 1927 +1
+ Partials 696 674 -22
|
Hi @samcoveney
We are currently in the release process, doing multiple checks, finalizing some important points, etc... I just want to let you know that we will start to look in detail at your PR next week, after the release. |
Any chance that this could get looked at soon? Let me know if there's anything I can do to help here. |
Hi @samcoveney , Apologize for not giving news. The core maintainers (including me) are really focused on this workshop that we are organizing: https://dipy.org/workshops/dipy-workshop-2022 That's why we were not so active these past 2 weeks. The workshop finishes this Friday, so everything will get back to normal, next week (review, update, etc...). |
Tests are passing on my system, are they failing here for reasons not related to my PR? |
yes, fixed in #2566 |
Hi @samcoveney, Can you share some data? I am sure I understand what you are trying to do. I got a general idea, but something is disturbing me. the Please, let me know where can I find cardiac MRI data. Thanks |
Hi @skoudoro thanks for taking a look. Open cardiac MRI datasets (for DWI) are hard to come by, which is why I showed the idea using slices of brain data. Can you take a look at the example file that I wrote? What is the problem with This may not be the best implementation of course, it was just meant to allow a simple extension of the API. It could be better to tell the user to simply provide an array of shape I appreciate your thoughts on this. |
I've not heard back on this, but I think I will modify the code to take out the |
1419292
to
ca6268a
Compare
5935e1e
to
765963e
Compare
The functions in dipy/align/_public, which conveniently allow to register entire dwi series, are only designed to work with 3D volumes, i.e. 4D data, in which the acquisitions are in the last dimension.
In cardiac MRI, data is often stored in a nifti file as if it were volumetric, but the third dimension corresponds to slice positions, i.e., [Nx, Ny, num_slices, num_acq]. These data for different slices were collected in separate acquisitions, so we cannot treat data[:, :, :, 1] as a volume. The registration needs to be performed on each slice separately, and this is a 2D registration problem.
I have adapted the code in dipy/align/_public to work in such a scenario by giving an optional argument "slice_index", which when provided causes the code to perform 2D registration on the given slice.
One tricky catch with 2D registration, is that the images need to be in the same plane. The 3D affine that can specify static_grid2world and moving_grid2world could be checked to ensure this. But then the resulting data would (in general) have 3D coordinates even if the data was effectively 2 dimensional. For this reason, it's easier just to work in pixel coordinates, and enforce that *_grid2world to np.eye(3) (note that this corresponds to 2D data, which is an additional reason for this change).
This pull request is work in progress, but it is pretty much complete. I would like some feedback please. There is an example provided.