Fibonacci patch #86
Merged
Fibonacci patch #86
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…per Fibonacci lattice
…alize axis selection in precess tool
Daval-G
added
bug
paquiteau
reviewed
Apr 4, 2024
|
Awesome stuff ! I have made small suggestions, but overall this is good for me :) |
|
Congrats @Daval-G for this awesome upgrade on handling 3D Cone trajectories! |
|
LGTM ! |
chaithyagr
pushed a commit
to chaithyagr/mri-nufft
that referenced
this pull request
Apr 11, 2024
* Fix wrong concatenation with initial gradients & slew rates * Clean FLORET trajectory, improve 3D cones & Seiffert spirals with proper Fibonacci lattice * Update examples * Add Fibonacci lattice functions, move maths functions into new file * Black & ruff * Fix tilt initialization errors with number.Number * Add math function to rotate around an arbitrary axis * Add Seiffert spiral rotation, Wave-CAIPI Fibonacci packing, and generalize axis selection in precess tool * Apply black & ruff, add missing docstrings * Update Wave-CAIPI examples, update init files to include tools * Update FLORET and Precess examples * Update 3D Cones and Conify examples * Update Seiffert spiral documentation and examples * Applied black & ruff for format * Integrate colinear rotations into Rodrigues'coeff function * Update precess documentation and fix minor bugs * Update and extend precess examples * Re-apply black & ruff check * Remove internal GCD function, check Real instead of Numbers --------- Co-authored-by: Guillaume DAVAL-FREROT <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR patches the bug from issue #72 and changes a few features. Most of them are related to the use of Fibonacci lattices, hence the name:
precesstool was previously introduced mainly to provide a way to rotate shots such as 3D Cones and Seiffert spirals following the Fibonacci sphere lattice approximation to provide a mostly uniform distribution of shots over the k-space sphere. However it was subtly wrong in the way polar angles were handled, as shown below with the clouds of points used to rotate single shots. It resulted in a greater density around the precession axis instead:Previously wrong method:
New correct method:
The reason was simply that the kz coordinate was handled in a polar fashion rather than an axial fashion, as shown below. Now both options are available, and the
precesstool is designed to provide a proper Fibonacci sphere distribution with its default arguments. The examples were also largely extended to better explain this tool.Previously wrong method (polar partition):
New correct method (axial partition):
maths.pyfile. TheRvfunction that returns a rotation matrix based on two vectors using Rodrigues' rotation coefficients now handles properly co-linear cases, and a new function has been added to provide rotation matrices around arbitrary vectors (used in Seiffert spirals).maths.pyto generate Fibonacci square, circle and sphere lattices. Those are used to approximate analytically a uniform distribution of shot positions/orientations, typically over a sphere surface.conifytool, which solves the problem signaled in issue Trajectory issue: Min value out of [-0.5,0.5] range #72, and the fixedprecesstool with proper Fibonacci sphere lattice.precesstool with proper Fibonacci sphere lattice, but also around their own axes to avoid azimuthal redundancy, following authors methodology.