fix up handling of bvalue scaling and bvec normalisation

[jdtournier]

OK, prompted by Chris Rorden, based on this discussion, I've had a look into our handling of the bvalue scaling, and it's fair to say that it's a bit messy at the moment...

To start off with, following #60, the DICOM import code and any subsequent conversion will leave the DW scheme unmodified: no normalisation of bvecs, no bvalue scaling. All those modifications happens in the app processing the data.

Where this is problematic is for 3rd party apps that rely on mrconvert for the DICOM to NIfTI conversion: if they expect the information to be usable as-is, they might come unstuck on e.g. multi-shell data acquired on a Siemens.

The other situation where this is annoying is when you have data where the bvalue scaling shouldn't happen: you then need to remember to pass the -bvalue_scaling false option every time you use a command that uses the DW encoding. There's no simple way to store the information as you expect it to be used, rather than as the scanner reported it.

So that argues for at least providing options to control that within mrconvert. We may also need control over normalisation of the bvecs without bvalue scaling (for instance, I just came across some Bruker data with non-unit vectors, but correct bvalues). At least that would provide users with some control over the interpretation of the DW table that they can apply once at DICOM import and then not have to worry about it, in those cases where it would otherwise be annoying.

But equally pressing is to sort out the glaring inconsistencies in how we handle these things currently. This is what I've observed from a multi-shell dataset that needs bvalue scaling:

---

mrinfo DICOM/ -dwgrad

shows the correct encoding (scaled by b-value), as it should.

---

mrconvert DICOM/ dwi.nii -export_grad_fsl bvecs bvals
mrconvert DICOM/ dwi.nii -export_grad_mrtrix enc.b

results in bvecs/bvals or an enc.b with no scaling by b-value, no unit normalisation of gradient vectors.

---

mrinfo DICOM/ -export_grad_fsl bvecs bvals
mrinfo DICOM/ -export_grad_mrtrix enc.b

results in bvecs/bvals or an enc.b with scaling by b-value, and unit normalised vectors.

---

mrinfo dwi.nii -fslgrad bvecs bvals -dwgrad
mrinfo dwi.nii -grad enc.b -dwgrad

both report whatever was stored in the corresponding bvecs/bvals or enc.b: no bvalue scaling, no unit normalisation. Yet this is supposed to report the DW encoding as it would be interpreted in our apps - which it gets right if run on the DICOM directly (see the first call above). I'm not even sure how that's possible, there must be some weird code path in there...

---

I'm not sure how to deal with this properly, but open to suggestions. I think at the very least, we need to ensure consistency in how the -export_grad_* options work, and in how the gradient table is interpreted, regardless of where it came from (DICOM headers or provided by -grad or -fslgrad). I also reckon we should add options to control the bvalue scaling and bvecs normalisation separately. What I'm not sure about is whether these should be provided as part of the generic DW gradient table import (or export?) option group, and whether they should be performed by default (which I'm a little reluctant to do, there was a reason we went along with #60). Thoughts welcome.

[bjeurissen]

This is the kind of trouble that is similar to what I have highlighted in #980. I am in favor of dealing with this once in mrconvert (with appropriate defaults and options to override), and then have all the subsequent commands rely on a well specified and consistent dw_scheme. Having each command deal with this is tedious for us and confusing and easy to mess up for users. If a power user still wants to see the original scheme, we should just keep it as a separate header entry in the mrconvert output. I am perfectly fine with the fact that this means that subsequent exports to other formats will lose the untouched dw_scheme. I think this is exactly what most users expect: a dw_scheme or bvals/bvecs combination that will work.

[thijsdhollander]

Not 100% sure I've fully understood the issue at hand (but I think I get it); but if I do, then I'm agreeing with @bjeurissen's general proposed strategy here:

I am in favor of dealing with this once in mrconvert (with appropriate defaults and options to override), and then have all the subsequent commands rely on a well specified and consistent dw_scheme.

I'm personally also fully ok with with the fact that

... this means that subsequent exports to other formats will lose the untouched dw_scheme. I think this is exactly what most users expect: a dw_scheme or bvals/bvecs combination that will work.

The primary goal here should probably be to avoid that "most" users would make, unknowingly, important mistakes. ...and even if something went wrong, it can then in most cases be traced back to a well identified point in their pipeline: the importing of their data (i.e. mrconvert). That'll make support much easier, and less confusing both for us as well as those users.

So well, 👍 from me for @bjeurissen's opinion on this one.

[jdtournier]

Yes, I'm also rapidly coming to similar conclusions. The problem though is how to handle externally-supplied encodings, either via -grad or -fslgrad, where we can't guarantee the correctness of the information. I think what we want is all the options for normalisation / rescaling in the GradImport options, but also provide the corrected gradient table in any output produced (either in output image header, or via the -export_grad_* options), rather than the original (which is the current default, and the point of #60 originally). So this would essentially revert #60, but add further control on the steps performed at import. Does that sound OK?

Also, how do we handle a gradient table if it's in the header? Presumably by that stage, it's already been fixed up and it's good to go as-is, so we don't want any of the gradient import options to apply. But how do we handle the -grad option? Do we by default assume it's in the correct format already, and also leave it untouched unless additional GradImport options are provided? What about bvecs/bvals supplied via -fslgrad? Do we also treat them as correct, and leave them unmodified unless modifications are explicitly requested? So this would mean that the DICOM import would perform different actions by default than the others?

Or do we assume that we should perform vector normalisation and b-value scaling by default every time? I reckon if the scheme has already been corrected, then these actions would leave everything as-is (although there is scope for very minor precision errors to creep in). So maybe the safest is to by default always apply both actions when importing from external sources (whether DICOM, or -grad/-fslgrad), but provide an option to disable these modifications. We would also leave any information found in the mif header unmodified...? Not sure about that last one, it might be tricky to achieve given how the code is organised.

Maybe another option would be to by default unit-normalise the vectors, but first check whether any of the vectors norms differ significantly from zero, and if they do, trigger a b-value rescale. This would avoid precision errors creeping into the b-values, I reckon. And also provide an opportunity to warn the user that the b-value rescaling is being applied, which might not be a bad thing.

What do you all reckon?

[Lestropie]

Check dwipreproc operation with bvecs / bvals: I'm thinking it could be provided with multi-shell data where, if scaled correctly, there is insufficient volumes to be considered as shells, but if erroneously not scaled, the number of volumes in the single shell is sufficient. If eddy actually succeeds, that would suggest that it's not itself been scaling b-values, and therefore internally the GP model may have been set up incorrectly.