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: Add streamline deformation example and workflow #1900
ENH: Add streamline deformation example and workflow #1900
Conversation
Hello @jhlegarreta, Thank you for updating !
Comment last updated at 2019-08-06 00:58:55 UTC |
@Garyfallidis @arokem @dPys @skoudoro comments and reviews are welcome. A few comments:
|
I guess we acknowledge that the |
Codecov Report
@@ Coverage Diff @@
## master #1900 +/- ##
=========================================
Coverage ? 83.09%
=========================================
Files ? 118
Lines ? 15016
Branches ? 2375
=========================================
Hits ? 12478
Misses ? 1972
Partials ? 566 |
Hey @dPys - could you take a look and try this out? |
As discussed in todays dev meeting, one of the things that needs some improvement is explaining the parameters of the |
1fa9380
to
b7b2400
Compare
Hi @arokem and @jhlegarreta , I have gone through the tutorial and made a number of modifications, but @jhlegarreta feel free to handle modifying the PR directly since you've already opened it:
And here's the final script (i.e. without the fetch of the FA template image added yet):
|
Thanks for the comments and suggestions @dPys. I will have another pass at it tonight. |
I've gone through it. Thanks Derek ! I've seen that you have posted the FA file. Nevertheless, in order to to make the fetcher, I would need the original URL where the FA of HCP1065 dataset can be retrieved from. Or else, did you get it querying FSL? @arokem do you happen to know where this data is located in the HCP project? Or else, will we need to store the file Derek posted in some UW server so as to make the fetcher? |
Hi @jhlegarreta , No problem. I'm not sure why I initially referred to the changes as "major" since they were actually minor and I just want you to know that you did a fantastic job of putting everything together. As for the source of the HCP FA template, I'm afraid that this is something that I've created custom for exactly this application. That is, the file is a resampled version of the 1mm FA template released from HCP1065 and that is included in the latest FSL template collection. For this reason, it will probably need to be hosted on a server that dipy can access for the fetch to work... One last thing-- the orientation and FOV of the moving image (the FA image/B0) has a big impact on the solution. I am working right now to determine what features of the affine are needed and how to make sure the code generalizes to weird formats. Stay tuned, |
Thanks Derek ! The HCP database requires a user/pwd as well, so hosting it on a server DIPY can access is the solution. |
Good to know. Thanks much. As an update, it also appears that in the case that the dwi data is stored in neurological format (as with the stanford hardi data), the registrations should just work as is. If, however the data is stored in RAS+ for some reason (this is enforced in ndmg and pynets for instance), then SyN + deformation appears to fail, and I'm not entirely sure why. One (band-aid) solution that I've found is to flip the FA image to neurological manually before running SyN (i.e. the flipped version becomes the moving image), and then use the accompanying flipped affine for the streamline deformation, but the original (i.e. unflipped) affine to transform the deformed streamlines to their final affine. In other words the non-linear warping and deformation all still occur in LAS, even thought the rigid-body transformations map the end result back to RAS. Here's an example where the tractography was done in RAS+ (stored in streams={path to a .trk file}):
|
Hi @jhlegarreta , After further tinkering this morning, it seems that all orientation issues could theoretically be addressed by setting the *_grid_to_world parameters for deform_streamlines to np.diag(np.array([2, 2, 2, 1])) where the signs along the diagonal of the affine are set adaptively based on the signs of the moving_affine and static_affine. I'm not sure that in all cases this will implicitly handle the offsets (which may have to be summed in some manner across moving and static affines regardless). @arokem -- would you be able to shed some light on how we might go about generalizing deform_streamlines to better work with any input image orientation? |
b7b2400
to
127279f
Compare
Rebased on master to adapt to the change in #1902. Force pushed inadvertently, and all previous commits were pushed again. I'll squash once we finish the example. |
Hi Jon, Thanks for merging the changes into the example. After playing more with this today, one thing that has become fairly clear is that a key assumption will need to be, as you noted previously, that the dwi data is stored in RAS orientation. The template image can be in LAS, as in the example right now, but if it is in RAS as well, then "adjusted_affine[0][3] = -adjusted_affine[0][3] - static_affine[0][3]” would instead become "adjusted_affine[0][3] = -adjusted_affine[0][3] + static_affine[0][3]” One other thing that I discovered is that instead of explicitly transforming the non-rigid deformed streamlines, another option is to perform the following:
The above approach should work, for instance, in the case that both the moving and static images are in RAS, and the offsets are scaled appropriately in the case that any image reslicing was performed. |
Thanks for the hard work @dPys. Looks like a less complicated solution than the previous one, and so more appropriate for an example (?). Let's see what others think. |
Yes @jhlegarreta -- this should be en route to a less complicated (albeit still incomplete) solution and better use of the affine parameters in deform_streamlines. Would love to hear others' feedback as well. Cheers, |
Hi everyone -- sorry for my slowness here. Unfortunately, I will continue to be rather slow to reply in the next few weeks, as I am organizing a summer school here in the next couple of weeks, and going on vacation after that. Regarding HCP data: the problem with using HCP in examples is that though the data is openly available, it is provided under the condition that users agree to certain terms and conditions. This means that we cannot redistribute HCP data. That's the reason we have the CENIR dataset in the first place. I am not sure whether this applies equally to population-level templates, though. If FSL is distributing it as part of their software distribution, maybe it's OK to distribute that? |
Hi Ariel-- No worries. Getting @jhlegarreta 's WIP to work smoothly has become a high priority of mine, and to be honest, it may take me a bit more time to tinker with the affines before we get there completely. In the long-run, these routines may eventually be more elegantly written out with their own class... One caveat to consider RE: the HCP template-- the 2mm template was actually something I created by resampling the 1mm template which is openly distributed by FSL since their latest release. No HCP 2mm template previously existed as far as I'm aware. Let me know if you still think it would be best to go with a different MNI WM template (i.e. one already available with fetch). Best of luck with hack school, teach em good! |
127279f
to
299521e
Compare
Rebased on master to fix conflicts and push forced.
😟 In that case, and unless a formal/official reply is received from the HCP consortium, I would not rely on what another software is providing. If we still want to go the FA way, although it may seem uncommon not to use the HCP as the reference, @dPys does any major drawback of using the CENIR dataset instead of the HCP dataset come into mind? Not sure that is an advantage, but the CENIR dataset's resolution is 2 mm isotropic. |
@jhlegarreta -- I just PR-ed the solution into the AddStreamlineRegistrationTutorial branch on your dipy fork (also found here: https://github.com/dPys/dipy/blob/AddStreamlineDeformationTutorial/doc/examples/streamline_registration.py). Let me know what you think :-) It should now work with all RAS+ oriented images. The CENIR dataset that you mentioned doesn't appear to have an FA template in MNI-space? For now, I'm still using the HCP 2mm FA template that I made, but when we find a suitable replacement that can be obtained with fetch, we could just swap out #L47-48. |
As always, thanks @dPys ! I've commented the PR you made.
Not that I know. If we finally need to use it instead of the HCP data, we'd need to compute it in the example itself I guess. |
Please see #1909 (comment). I think that it would be fine to have a less-than-perfect example here, and then explain in the accompanying text how you would improve upon it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this nice tutorial @jhlegarreta and @dPys. Here a quick review but I need to run it first.
Hi @arokem , Sounds good. Do you have another FA template in mind for us to use instead of the HCP one? |
I'll do the rebase. In order to do that, a decision on the dataset issue would be welcome. As I see things, we have two choices:
Not sure about the amount of code the latter would require, but I guess it fits within the purpose of the example. I am fine with either approach. |
Thanks @jhlegarreta ! What about using the FMRIB58_FA_1mm.nii.gz image from fsl/data/standard (also attached), It's pretty legacy, but at least it's an FA template (it's also the one typically used for tbss), and at least it's not from HCP... |
I guess the FSL license is again something to look at: No part of the Software may be reproduced, modified, transmitted or transferred in any form or by any means, electronic or mechanical, without the express permission of the University. The permission of the University is not required if the said reproduction, modification, transmission or transference is done without financial return, the conditions of this Licence are imposed upon the receiver of the product, and all original and amended source code is included in any transmitted product. You may be held legally responsible for any copyright infringement that is caused or encouraged by your failure to abide by these terms and conditions. Not sure if DIPY is comfortable with this as it seems to refer to the SW rather than the data ((...) and all original and amended source code is included in any transmitted product.). |
Thanks for looking into these options with me @jhlegarreta . I suppose the stanford B0 template will just have to work for the time-being just to get this thing out there like you and @arokem suggested (i.e. approach 1)? Sounds like our next task may be to create a new FA template image for dipy :-) Although I don't have strong empirical justification for it for direct streamline normalization, I know that @clintg6 and @mattcieslak would probably agree that in the long-term, an FA template image would be ideal. Great working with you @jhlegarreta, and thanks for helping me to refine the example so well! |
Add streamline deformation example and workflow. Resolves dipy#1840.
Address PEP8 warnings.
Add references to the SyN paper and ANTs software.
Incorporate @dPys' suggestions: - Use the FA map as the template. - First apply the non-rigid registration, then the affine transformation with corrected extents.
0eace82
to
f66eb9c
Compare
Use the mean b0 template back and rebase on `master`. Use the new `load_tractogram` and `save_tractogram` methods to encourage their adoption and increase consistency for the future. Complete the documentation. Increase consistency when passing string literals to methods.
f66eb9c
to
d614b88
Compare
Push-forced. Rebased on Not done, though. The streamlines are not located at the right place; I guess the affines part is not correct 😔. |
Hi @jhlegarreta -- please see my PR into your forked branch which includes some revisions: jhlegarreta#3 Basically, the following line was missing:
But also, the img_t2_mni data needed to be resliced to 2mm voxels to match that of the HARDI data: and it appeared that the affine_map was instead being set from an identity matrix which won't work with the affine flipping code. There was also a caveat with the img_t2_mni template image that was not a caveat previously with the HCP FA template image:
Ultimately, there are three core assumptions for direct streamline normalization to work as expected with the routines now set up with dipy:
And viola: Also, it may be worth overtly stating the limitation of using a t2-contrast template image as the target for normalization. Namely, it does not provide optimal tissue contrast for maximal SyN performance (see SyN axial slices below). Thus, ideally, an FA template should be used as the static image, and an FA map of the moving image should be used as the moving image. |
Update streamline_registration.py tutorial to work with B0 template data
143fa37
to
cb661da
Compare
Make the example style be consistent.
Thanks @dPys for enlightening the way !
Added these in 4c0718d. Importing Thus, the example is working now 🎆 🎉 🍾 . If we want to change the files' name to honor the title given, and/or change the axial slice view to a sagittal one, now it's the time. Otherwise, if maintainers @arokem @skoudoro @Garyfallidis give it green light, it can be squashed (to avoid populating the history with some unnecessary comings and going commits) and merged ! |
I'd be supportive of renaming the file to streamline_normalization.py but I'm happy with whatever everyone thinks is best. This was a fun one, and there is still plenty of room for refinement down the road. Great working with you @jhlegarreta and looking forward to working with you again! |
@jhlegarreta -- I've simplified the affine transformation code even further. @arokem may be interested in this solution as well. Recall that in addition to running SyN, we need to account for the origin offsets in the FOV from both the template image (static) and B0/FA image (moving) using transform_origins. Let's use this origin transformation (that defaults to identity along the diagonal) as the starting point for the current_grid_to_world transformation that we can later feed into deform streamlines:
Now, what we are going to do is make two small adjustments to y-dimension of the origin transformation above: 1) we need to flip the sign of the offset to get the mirror image of transformation (we don't need to do this for the x-offset because as you will see, we are going to 'ignore' this dimension later anyway); 2) we next need to divide the offset by the square of the voxel resolution (i.e. since the diagonal was merely an identity matrix and did not account for the image zooms-- here, 2mm). We use the square of the zoom (i.e. 4) because the origin transformation in the y- and z-dimensions both need to be corrected (again, we are ignoring the x-offset) and we are working in 3-dimensional space:
Once that's done, then all that's left is to deform the streamlines by applying the forward deformation field. Now, the deformation field is not aligned with the FOV of the streamlines, so the one way to handle this is to simply collapse the first axis of the field, which will ensure that it is anchored to the corner of the FOV in the x-dimension when it is applied (this is also why we can avoid having to adjust the x-offset mentioned earlier):
And that's it! Hopefully this streamlines things a bit more ;) |
@dPys thanks for the endeavor to simplify things and explain them thoroughly ! I will not be able to test or edit the code for a few weeks, and will have limited/intermittent network access, so go ahead with adding commits to this PR if necessary. I am fine with either of the last two versions, but it is true that your last proposal is more compact. Otherwise, this is ready to be merged on my end ! |
Checks are passing so the code is ready to be merged if no further modifications are required. |
Feel free to merge!
…Sent from my iPhone
On Aug 30, 2019, at 5:45 PM, Jon Haitz Legarreta Gorroño ***@***.***> wrote:
Checks are passing so the code is ready to be merged if no further modifications are required.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
I think this is ready to go! Thanks for all your work @dPys and @jhlegarreta! |
Add streamline deformation example and workflow.
Resolves #1840.