-
Notifications
You must be signed in to change notification settings - Fork 185
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
dwifslpreproc: MIF files don't retain gradient table #2577
Comments
Ouch, that doesn't sound good... Can you provide the exact commands that cause the issue? And if possible, run |
Log file attached. Let me know how you want the folder from Input file:
Command: Output:
|
OK, that's odd... The last command to run is:
which happily reports:
Yet the output image seems not to contain any DW gradient scheme... Can you post the output of:
Just to check that this image did indeed contain a valid DW scheme...? Right now, I'm struggling to see what could have caused that... Any ideas, @Lestropie...? |
Information below. Interestingly, I can run the very final Also the nans seem like they'll be a problem.
And for completeness: bvecs:
dwi_post_eddy.eddy_rotated_bvecs
|
I'll try to replicate the configuration using my own data rather than having to download yours. First thing that takes my eye is:
So perhaps wherever the NaNs are creeping in, getting that data into Python via the JSON import is converting them to
Is this the Between |
I have a suspicion the NaNs might be due to those b=0 images not being recognised as b=0 as they're actually b=19.5 - which is above our default threshold. I wonder if things might actually work out of the box if you add:
to your |
Hi @Lestropie, So, I can address some of these.
Notables:
Questions:
What do you mean by "null vectors" in this case? I'm assuming it's the [0 0 0] vectors you're referring to (since that's where the nans end up), but I guess I don't understand how you'd then represent b= 0 if its not [0 0 0]. |
That's at the point of import. My suspicion is that in
Well even at our end it's only fairly recently that we've supported |
First finding: problem only manifests in the case where it is the case that both the vector direction is null and the b-value is non-zero. |
A I think the solution here is to prevent Edit: An alternative is that if a |
PS.
This relates to c7c1cc9 in #1603. The final image export step of Python scripts involves taking template image header contents and importing it into the exported image. This means in particular that (There's a separate argument to be had over whether some of those modifications should or should not be done, but let's defer that for now...) The JSON for Modern C++ library that we use for JSON handling, when it sees the NaNs in the gradient table, instead writes them to the JSON as " |
OK, thanks for the deep dive, Rob!
Yes, that's my immediate gut feeling also. We would however need to double-check what happens with trace-weighted data, which some manufacturers have a habit of producing with zero DW direction vectors and non-zero b-value. But I expect as long as we only zero b-values <BZeroThreshold, that should be OK too, right?
Should we treat 'null' as equivalent to NaN when parsing these values, then? I think it should just be a matter of adding to the list at this point in the code... (?) |
Thanks for all of this. As a wrap-up for me and my data need in the short-term, should I just downgrade to |
To be clear, the two alternatives are:
Neither of these are ideal, but my suspicion is that while either may be a solution for
Personally I think no. In Python there's a considerable distinction between a floating-point NaN and @araikes: Easier would be to manually modify your gradient table to explicitly have zeroes in the fourth column for volumes intended to be b=0. Then |
Looking at the docs, this looks very much like the intended behaviour. I don't see that being patched any time soon... As to how to handle things going forward, personally I think option 2 is the most likely to cause problems, since this gives a direction where there was none, and invites eddy (or MRtrix) to treat these as a b>0 shell - which if anything is likely to glitch if it tries to fit SH to it. I think these need to be zero'd to make it very clear to eddy (and other downstream applications) that these are to be interpreted as b=0 volumes. This is also what the previous version did (I think?). Also worth linking with this discussion on the forum. Not the same exactly, but it's probably worth expanding the fix to also force any entry with b<BZeroThreshold to zero while we're at it. |
The converse I was thinking of was IVIM, where loss of non-zero but small b-values could be a problem. But surely users of such would be manually specifying a smaller
Request already exists in #2495.
As in, even if the vector is valid? |
OK, ignore that discussion on the forum, I got myself mixed up... You're right, #2495 is probably the right answer there.
I would expect IVIM to explicitly ignore |
In the IVIM implementation itself, of course. But this issue is around |
I assume that you (@Lestropie and @jdtournier) arrived a solution that is going to work for MRtrix and the majority of use cases, however just some additional testing that I finally had opportunity to do with these data:
Just as an aside, other tools (e.g., |
We didn't decide definitively on one. I'm confident that the solution should be explicitly limited to the
OK, I think that conflicts with my own prior testing. What I might do is create a branch where on |
Where the gradient direction vector is zero, but the b-value is non-zero, when FSL's "eddy" attempts to rotate the directions of b>0 volumes according to subject rotation, this results in corrupted gradient directions. This change prevents such data from reaching "eddy" within dwifslpreproc, by detecting volumes where the gradient vector is zero and the b-value is non-zero but is classified by MRtrix3 as non-zero based on BZeroThreshold and forcing the value within the bvals file to be zero. Relates to #2577.
Where the gradient direction vector is zero, but the b-value is non-zero, when FSL's "eddy" attempts to rotate the directions of b>0 volumes according to subject rotation, this results in corrupted gradient directions. This change prevents such data from reaching "eddy" within dwifslpreproc, by detecting volumes where the gradient vector is zero and the b-value is non-zero but is classified by MRtrix3 as non-zero based on BZeroThreshold and forcing the value within the bvals file to be zero. Relates to #2577.
Hello. Seeing as this issue is still open, I'll just throw something else in. It seems that
Tested on mrtrix 3.0.4, with fsl 6.0.7.1 and 6.0.7.4. |
@lconcha Presumably you're applying your hack after the code block corresponding to old versions of Did you happen to try the code in #2602? That's intended to condition the |
Yes, I placed that |
I've expanded the scope of #2602 via bac6028 to include sanitisation of bvecs / bvals import that may contain NaN values. That should hopefully make the MRtrix3 software robust to whatever FSL generates, regardless of the condition of the data that was provided to FSL (which may not necessarily come from |
I have data that I've processed (successfully) using
v 3.0.3
. The data is converted to MIF with the gradient table and passed todwifslpreproc
and thendwibiascorrect
without issue. I upgraded to 3.0.4 and get an error that there is no gradient table ineddy_corrected.mif
. I tried usingdwifslpreproc ... -export_grad_fsl ...
in order to export the gradient table but it also throws and error about the table being missing. This appears to be unique tov. 3.0.4
The text was updated successfully, but these errors were encountered: