-
Notifications
You must be signed in to change notification settings - Fork 180
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
New classes for fixel dataset handling #2657
Draft
Lestropie
wants to merge
5
commits into
fixel_dataset_handling
Choose a base branch
from
fixel_dataset_class_sift
base: fixel_dataset_handling
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
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
Major changes: - New class MR::Fixel::IndexImage, which centralises handling of the index image in the fixel directory format. - New class MR::Fixel::Dataset, which extends the Minor changes: - Function MR::Fixel::copy_index_and_directions_files() renamed to MR::Fixel::copy_all_integral_files() to reflect inclusion of dixelmasks image if present. - In class Math::Sphere::Set::CartesianWithAdjacency, in order to resolve ambiguity in functor operation, Adjacency is now a member variable rather than a derivative class. - TrackMapperBase now has a FixelMappingPlugin, which allows direct mapping of a streamline to a set of fixels. - Mapping::TODMappingPlugin is no longer thread-safe, but instead keeps a vector of data to store SH coefficients as a member variable; TrackMapperBase correspondingly keeps a std::unique_ptr<> rather than a std::shared_ptr<>. - Fix to TrackMapperBase::voxelise_precise() to handle case where streamline endpoint lay at a voxel face. - Mapping::Voxel no longer contains length as a member variable, but instead inherits from new class IntersectionLength; this is because new class Mapping::Fixel also needs to store an intersection length, but unlike other variations, does not inherit from Mapping::Voxel as an Eigen::Vector3i.
- Modifications to MR::Helper::ConstRow and core/eigen_plugins/dense_base.h to permit loading an array column from an image row. - Full renaming of SIFT "processing mask" to "model weights". - Move code responsible for resampling of ACT 5TT image (prior to use of such to calculate model weights) from SIFT to ACT namespace. - Allow user-provided SIFT model weights to be fixel data file, image volume (either on the same voxel grid as the fixel dataset or no), or be inferred from the ACT 5TT image.
Super excited about the changes to |
5 tasks
Lestropie
added a commit
that referenced
this pull request
Jul 16, 2023
Due to changes in #2639, the DWI::FMLS class no longer populates the fixel lookup table per voxel. However the DWI::Fixel_map class was still dependent on this information. In this commit, the DWI::Fixel_map class constructs that lookup table locally in order to facilitate compilation processing. Note however that results will not be identical to that of version 3.0.x, since this version of the LUT construction does not include dilation to allocate all directions to the nearest fixel, as is the default behaviour in that version of the software. Further, this change will become redundant with the changes in #2657.
Conflicts: core/math/sphere/set/assigner.cpp core/math/sphere/set/assigner.h src/dwi/fixel_map.h src/dwi/fmls.cpp
This was referenced Aug 14, 2024
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 is part of what the set of changes aggregated in #2644 have been working towards.
At time of initial posting the changes apply only to
tcksift
andtcksift2
, but that list will increase over time.Rather than receiving as input an FOD image and performing FOD segmentation at commencement of the command, they will instead take as input a fixel data file encoding fibre density, and automatically load all other information relevant to the fixel dataset.
It has taken quite some time since the fixel directory format superseded the initial sparse fixel file format for me to get around to addressing how it conflicts with both pre-existing handling of, and intentions for future developments involving, fixel data.
Previously, these preferable attributes were achieved using the
DWI::Fixel_map<>
class and derivatives.This predated the fixel directory format, the legacy fixel file format, and even the term "fixel".
But it:
The primary goal of this PR is to supersede the
DWI::Fixel_map<>
class with newMR::Fixel::Dataset
.This class is more consistent with the philosophy behind the fixel directory format.
However it also provides the relevant quantitative information per fixel in a native, memory-contiguous manner.
Further, it maintains utilisation of the dixel mask information in assigning streamlines to fixels; this was a key advantage of keeping the relevant commands utilising FOD segmentation rather than the reduced resolution of fixel mean direction information only.
PR is in a highly preliminary state; intend to branch other developments off of here onto a private repository.
Below is list of items that should largely be addressed before merging to base branch of #2644.
afdconnectivity
: Take fixel FD file as inputfixelconnectivity
: Make use ofTrackMapperBase
's newFixelMappingPlugin
tckgen
: Have dynamic seeding take fixel FD file as inputtckmap
: Integrate fixel dataset target destinationtck2fixel
entirelyFORCE_INLINE
SIFT::ModelBase::FixelBase
class and derivatives to grabConstRowXpr
s for fixel direction and dixelmaskSIFT::Model
class and derivativesMR::Fixel::IndexImage
provides a looping construct that yields an iterable list of integer indices, this class should be able to do the same for an iterable list of the relevant Fixel class(possibly automatically skipping over any fixels for which the model weight is zero)
(I spent ages trying to get this working without success...)
Image<index_type>
as a fixel index image in favour ofMR::Fixel::IndexImage
In many circumstances it will not be desirable to construct a full
MR::Fixel::Dataset
instance.However rather than MR::Fixel::Loop in its current form, it may be preferable to instead modify that class to be initialised exclusively from an MR::Fixel::IndexImage.
Currently involves boolean check for presence of dixel masks on a per-streamline-voxel-intersection basis; would prefer to reduce the number of unnecessary branch executions
Optionals:
afdconnectivity
,tcksift
andtcksift2
: Allow user-specified input to be either an FOD image or a fixel FD file.This would require the ability to populate
Fixel::Dataset
from the contents ofDWI::FMLS::FOD_lobes
. This is conceptually not too difficult, and indeed the existing population ofDWI::Fixel_map<>
does something very similar. The catch is that the total number of fixels is not known a priori, andEigen::Array<>
is very expensive to do repeated.conservativeResize()
calls (as we found previously withamp2response
: amp2response speedup #1406). So what would be necessary is either:std::vector<>
's and populate theEigen::Array<>
's upon competion of segmentation; or:Fixel::Dataset
members, with a.resize()
to fit just the populated data at completion of segmentation.This is a capability that exists within the current SIFT(2) implementation, is disabled by default, but I don't quite know what I want to do about it going forward.
In its pre-existing form, this creates one additional "fixel" per voxel, that "claims" all directions not ascribed to any segmented fixel, and has an FD of zero. In this way, streamlines for which the tangent does not lie within the subset of the sphere corresponding to any fixel are assigned to this "extra" fixel, and always contribute to an excess in reconstructed density there. It may be possible to do something a bit more clever, for instance by comparing the total AFD of all retained fixels to the FOD l=0 term and attributing the difference between them to all directions not attributed to any fixel. But there's still uncertainty around how such data should be used in a downstream optimisation, rather than just assigning all streamlines to the nearest fixel (which is the current default behaviour).
There's also the question of whether this should be handled at processing time, or whether fod2fixel could include such data in the fixel output directory, with the corresponding dixel mask but with a non-finite direction. All downstream applications would need to be tested for robustness against the latter case.
A known issue with SIFT compared to COMMIT is the model fitting in areas of partial volume between GM and WM.
With COMMIT, a separate forward model component can be provided for GM, and in partial volume voxels the optimiser can choose how much of the DWI signal to attribute to streamlines vs. GM.
With SIFT, there is a kind of baked-in presumption of correspondence between the proportion of the voxel traversed by streamlines (which itself is likely influenced by ACT), and the proportion of the DWI signal attributed to WM vs. GM by MSMT (and that's not further accounting for which fixels may vs. may not have been successfully tracked).
This can introduce large model errors at partial volume voxels, which can undesirably drive the weights of individual streamlines very high.
While I have various tricks such as downregulation of such fixels in the model and regularisation of streamline weights, it remains a weakness.
What I would prefer is something that better takes into account the presence of non-WM tissue, whether from MSMT or from a 5TT image. While I haven't figured out yet exactly how I would want this to work, I think it should be possible to do something to stabilise the fit.