-
Notifications
You must be signed in to change notification settings - Fork 16
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
Integrate code from regtricks? #100
Comments
@oesteban Hello there - not sure if you've seen the above, and not sure if this was the best way to approach you. Hope you find this interesting. |
@tomfrankkirk Hi Tom, happy to chat more. This project though is envisioned to be merged into nibabel and to be general (not just FSL). Most of the features you mention are already implemented here, or will be shortly. Please let me know how do you think both projects can be coordinated. |
Hi Oscar,
"Aimed to be general, not just FSL, and merge into nibabel": yes, I am in complete agreement. At the moment I have only written the interfaces for FSL, but as I am sure you understand, the underlying logic is common regardless of what toolchain you interface with (AFNI, ANTs etc). I don’t have enough experience with the others to write interfaces for them. As for merging into a larger project, that is fine by me if it means the code available for a wider audience.
That said, interfacing with the non-linear registration component of FSL (FNIRT etc) has been tricky and, having spoken with Jesper Andersson about this, I have not re-implemented the underlying code that maps from FNIRT coefficient files to displacement fields (via many convolutions). Instead I make calls to fnirtfileutils to handle that for me, with some fiddly tricks to speed things up. I imagine similar issues may have arisen for the non-linear parts of other toolchains?
"Most of the features are already implemented, or will be shortly": for those that are not yet implemented, if they exist in regtricks already, then would it be helpful to merge those parts in?
Tom
… On 20 Aug 2020, at 14:45, Oscar Esteban ***@***.***> wrote:
@tomfrankkirk <https://github.com/tomfrankkirk> Hi Tom, happy to chat more. This project though is envisioned to be merged into nibabel and to be general (not just FSL).
Most of the features you mention are already implemented here, or will be shortly. Please let me know how do you think both projects can be coordinated.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#100 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AH2STTMJSBLNOKHVC6QYKZDSBUSHPANCNFSM4QDPJ5QA>.
|
NiTransforms (or NiBabel, FWIW) should be as light in dependencies as possible. Even more so for third-party libraries not in the Python ecosystem.
As long as those integrations comply with NiTransforms and NiBabel principles and code style, of course. It would be helpful to identify exactly what features have potential for the merge. Some other interesting option would be that you had a quick look at the open issues, and maybe make the connections with regtricks where there's overlap. I believe one area where this collaboration could work very well is in testing, as NiTransforms is following a test-driven development model. |
Yep, I'd love to get rid of the dependency on fnirtfileutils, but alas it hasn't been easy so far. I put quite a few hours into it without getting anywhere so decided to draw a line under that for now. I can go into detail if you want; suffice to say the relevant FSL code is pretty dense and not self-explanatory. One half-way solution that was suggested to me is to wrap around the relevant C++ code from FSL itself and build that in, however then you need to include some of the external libraries FSL depends on, so you've not really removed the dependency, just carried it in closer. Ok, I'll take a look through the open issues and report back. |
@oesteban Issue #89: extend the collapse method for a transform chain to include non-linear. I took a different approach to chaining transforms: although the concept exists in my code, I don't actually implement it as a specific class but instead use recursion and type promotion. The general idea is:
using 1) and 2), any chain of transformations is evaluated immediately and returned as a new transform object. In effect, the collapse() call is performed straight away and the 'chain of transforms' exists only in the calling scope (as the actual list that the user provides to chain()). the only caveat to the above is that my implementation is perhaps FSL-specific for the non-linear components (in particular, the concept of a pre-mat and post-mat). I don't know if other non-linear tools make use of the same concepts and therefore if it would port over nicely. |
Looking forward to contributions in the form of pull requests - thanks much! |
Hi, I've been pointed here by a user not on GitHub. I've been working on a toolbox that overlaps with your aims on this project, especially with regards to interfacing with FSL. This is it: https://github.com/tomfrankkirk/regtricks
The aim is to allow for easy combining and application of registration transforms, without replacing the process of finding registrations themselves. As of yet, it interfaces with the main FSL tools: FLIRT, MCFLIRT, FNIRT
Internally, it converts all registrations into world-world convention (as is the case for BIDS X5, for which there is some support built in) and then uses scipy to perform all interpolation / resampling operations. You can chain together an arbitrary number of transformations and it will resolve them into a single overall transformation object that is applied with one interpolation step. The user has full control over this if they want it.
So, in summary, its an elaborate wrapper for reading in FSL transforms and passing them off to scipy's interpolation libraries. If people were willing to help add code to interface with other toolboxes (ANTS, SPM etc) then I think all the underlying logic for combining / applying transforms would still hold true, and hopefully thats a step in the right direction for your overall aims. Equally you're very welcome to pull code out of this if helpful.
Let me know if you'd like to talk more.
tom
The text was updated successfully, but these errors were encountered: