dwi2adc: Change IVIM interface #3044
Conversation
Rather than concatenating all parameters in a single 4D output image series, the command now exports one or more 3D images encoding different parameters. Importantly this also means that the compulsory positional argument output of the command is the ADC, as expected from the command name.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
cpp/cmd/dwi2adc.cpp
Outdated
| logszero_and_adc = bsubinv * dwisub; | ||
| } | ||
|
|
||
| adc_image.value() = logszero_and_adc[1]; |
There was a problem hiding this comment.
warning: narrowing conversion from 'Scalar' (aka 'double') to 'value_type' (aka 'float') [bugprone-narrowing-conversions]
}
^
cpp/cmd/dwi2adc.cpp
Outdated
| if (std::isnan(ivim_cutoff)) { | ||
| if (szero_image.valid()) { | ||
| assign_pos_of(adc_image).to(szero_image); | ||
| szero_image.value() = std::exp(logszero_and_adc[0]); |
There was a problem hiding this comment.
warning: narrowing conversion from 'double' to 'value_type' (aka 'float') [bugprone-narrowing-conversions]
e);
^| const double f = C / S0; | ||
| if (szero_image.valid()) { | ||
| assign_pos_of(adc_image).to(szero_image); | ||
| szero_image.value() = S0; | ||
| } |
There was a problem hiding this comment.
warning: narrowing conversion from 'double' to 'value_type' (aka 'float') [bugprone-narrowing-conversions]
S0;
^
cpp/cmd/dwi2adc.cpp
Outdated
| } | ||
| assign_pos_of(adc_image).to(ivimfrac_image, ivimdiff_image); | ||
| ivimfrac_image.value() = f; | ||
| ivimdiff_image.value() = Dstar; |
There was a problem hiding this comment.
warning: narrowing conversion from 'double' to 'value_type' (aka 'float') [bugprone-narrowing-conversions]
f;
^
cpp/cmd/dwi2adc.cpp
Outdated
| } | ||
| assign_pos_of(adc_image).to(ivimfrac_image, ivimdiff_image); | ||
| ivimfrac_image.value() = f; | ||
| ivimdiff_image.value() = Dstar; |
There was a problem hiding this comment.
warning: narrowing conversion from 'double' to 'value_type' (aka 'float') [bugprone-narrowing-conversions]
f;
^| Eigen::MatrixXd binv, bsubinv; | ||
| std::vector<size_t> idx; | ||
| const Header H; | ||
| Eigen::VectorXd bvals, dwi, dwisub, logszero_and_adc; |
There was a problem hiding this comment.
warning: member 'H' of type 'const Header' is const qualified [cppcoreguidelines-avoid-const-or-ref-data-members]
H;
^
cpp/cmd/dwi2adc.cpp
Outdated
| if (opt.size()) | ||
| functor.set_bzero_path(opt[0][0]); |
There was a problem hiding this comment.
warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
| if (opt.size()) | |
| functor.set_bzero_path(opt[0][0]); | |
| "); | |
| ())!opt.empty() |
Additional context
/usr/include/c++/12/bits/stl_vector.h:1082: method 'vector'::empty() defined here
empty() const _GLIBCXX_NOEXCEPT
^
cpp/cmd/dwi2adc.cpp
Outdated
| header.ndim() = 4; | ||
| header.size(3) = ivim ? 4 : 2; | ||
| auto opt = get_options("szero"); | ||
| if (opt.size()) |
There was a problem hiding this comment.
warning: implicit conversion 'size_type' (aka 'unsigned long') -> bool [readability-implicit-bool-conversion]
cpp/cmd/dwi2adc.cpp:201:
- ())
+ () != 0u)
cpp/cmd/dwi2adc.cpp
Outdated
| if (opt.size()) | ||
| functor.initialise_ivim(opt[0][0], opt[0][1], get_option_value("cutoff", ivim_cutoff_default)); |
There was a problem hiding this comment.
warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
| if (opt.size()) | |
| functor.initialise_ivim(opt[0][0], opt[0][1], get_option_value("cutoff", ivim_cutoff_default)); | |
| "); | |
| ())!opt.empty() |
Additional context
/usr/include/c++/12/bits/stl_vector.h:1082: method 'vector'::empty() defined here
empty() const _GLIBCXX_NOEXCEPT
^
cpp/cmd/dwi2adc.cpp
Outdated
| if (opt.size()) | ||
| functor.set_bzero_path(opt[0][0]); | ||
| opt = get_options("ivim"); | ||
| if (opt.size()) |
There was a problem hiding this comment.
warning: implicit conversion 'size_type' (aka 'unsigned long') -> bool [readability-implicit-bool-conversion]
cpp/cmd/dwi2adc.cpp:204:
- ())
+ () != 0u)
As discussed in #2578.
After trialling the IVIM fit in
dwi2adcfor a facility user, I had to change the interface out of shear frustration.