Merge 3.0.5 into dev #3101

Lestropie · 2025-05-21T04:57:25Z

This is on the clunkier side because of dev now having both a mass reformatting of code and a re-arrangement on the file system. git now outright misses a lot of changes. So there will need to be extra diligence to make sure that changes applied to master prior to 3.0.5 have not been lost in the merging process. Assistance in that for PRs that are not my own would be appreciated.

Certainly the DWI metadata handling changes were initially problematic. On my first merge attempt the changes in #3011 disappeared. So what I have done is started with #2917, first brought that up to date with current dev, and then merged 3.0.5 onto that, knowing that much of the changes applied to master in #3011 were already manually applied based on the contents of #2917. This PR therefore supersedes #2917. I will however need to manually check the contents of #3011 and make sure nothing gets left out.

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.

If possible, use fsl_sub command to halt execution until all jobs have completed. Result of discussion in #2597. Addresses #2595. Replicates some contents of bd3f19e.

Mismatch between option string as loaded in usage() and the get_options() call results in user-specified option being ignored. Error introduced in b435e58 as part of #1932.

…ith unsupported transfer syntax This at least allow mrinfo to run. Any operation that attempts to access the voxel data will of course fail.

Co-authored-by: Robert Smith <[email protected]>

Uses an alternative design to f97d3ca / #2768 for supporting complex numbers in the reslice adapter. Specifically in determining the type to use during summation, f97d3ca chooses complex double if the type is not arithmetic, which could hypothetically lead to unexpected behaviour with unexpected template types. This alternative instead utilises the pre-existing is_complex<> SFINAE capability.

…read_with_unsupported_transfer_syntax

If the phase encoding information present in the header is only the phase encoding direction, but not readout time is present, then the constructed phase encoding matrix will possess only three columns, not four. If this happens, but an attempt is then made to cross-reference the readout time in the phase encoding table generated from header content against what the user has specified at the command-line, then an unhandled exception will occur. This change prevents specifically the comparison of readout times from being made if no such information is present. Replacement for erroneous commit faf0788. Also includes additional test that results in an unhandled exception on 3.0.4 but executes successfully with this change.

Whitespace present in name of associated compulsory argument erroneously implies that there are two input arguments expected.

Where a command receives a long list of inputs, while collapsing that set may be preferable from the perspective of user terminal information, it should not be hidden from a raw log of the set of commands executed. This change is not currently of high consequence as there is no mechanism that reads from that log file.

…ransfer_syntax

Adapter::Reslice: Alternative resolution of complex summation

Reslice adapter: allow use with complex numbers

Lestropie · 2025-09-02T05:57:09Z

- #2968: Absent - #3049: Absent - #2767: Partially absent - # 3027: Almost all absent (the new tests were in place but that was all) - #3047: Absent - #3071: Absent - #3011: Fill in gaps of changes that were applied to #3011 prior to its derivation from #2917. - #2609: Fixed a couple of small omissions. - #2602: Bits and pieces missing.

- Increment version number in CMakeLists.txt to reflect #3101 doing a merge with tag 3.0.5. - Various code features overlooked during writing of 1ebdb39. - Change MR::Axes::permutations_type to be a class rather than typedef in order to define a default uninitialised value, and also make it uint8 rather than size_t given it only has to stores the values 0, 1, 2, and an invalid flag. - Fix check for shuffle being the identity matrix as per #3169.

… sequence Re-application of f148613 from #3047 as erroneously omitted from 1ebdb39 as part of #3101.

- dwifslpreproc: Fix F-string with no variable resolution. - mrtrix3.fsl: Update to reflect move from path.name_temporary() to utils.name_temporary(). - Move new test dwscheme_preservation_imprecision from #3027, duplicated for 3.1.0 in 568b28e, to binaries/mrcalc directory: the test is not specific to mrcalc, and so had been placed in the unit_tests/ directory, but it depends on data only available in the binary test data repository.

Some of the attempted manual manipulation of string multi-line formatting either did not work as intended, or yielded a string overflow at compile time for certain compilers.

github-actions

clang-tidy made some suggestions

github-actions · 2025-09-03T12:35:39Z

cpp/core/dwi/gradient.cpp

-    bvecs.row(0) = -bvecs.row(0);
+  if (header.realignment().orig_transform().linear().determinant() > 0.0)
+    bvecs.row(0) *= -1.0;
+  Eigen::MatrixXd grad(bvecs.cols(), 4);


warning: implicit conversion bool -> 'ssize_t' (aka 'long') [readability-implicit-bool-conversion]

Suggested change

Eigen::MatrixXd grad(bvecs.cols(), 4);

ssize_t nans_present_bvecs = 0;

github-actions · 2025-09-03T12:35:39Z

cpp/core/dwi/gradient.cpp

+  if (header.realignment().orig_transform().linear().determinant() > 0.0)
+    bvecs.row(0) *= -1.0;
+  Eigen::MatrixXd grad(bvecs.cols(), 4);
+  grad.leftCols<3>().transpose() = header.realignment().orig_transform().linear() * bvecs;


warning: implicit conversion bool -> 'ssize_t' (aka 'long') [readability-implicit-bool-conversion]

Suggested change

grad.leftCols<3>().transpose() = header.realignment().orig_transform().linear() * bvecs;

ssize_t nans_present_bvals = 0;

github-actions · 2025-09-03T12:35:39Z

cpp/core/dwi/gradient.cpp

+    bool zero_row = false;
+    if (std::isnan(grad(n, 3))) {
+      if (grad.block<1, 3>(n, 0).squaredNorm() > 0.0)
+        // clang-format off


warning: implicit conversion bool -> 'ssize_t' (aka 'long') [readability-implicit-bool-conversion]

Suggested change

// clang-format off

nans_present_bvals = 1;

github-actions · 2025-09-03T12:35:39Z

cpp/core/dwi/gradient.cpp

+    }
+    if (grad.block<1, 3>(n, 0).hasNaN()) {
+      if (grad(n, 3) > 0.0)
+        // clang-format off


warning: implicit conversion bool -> 'ssize_t' (aka 'long') [readability-implicit-bool-conversion]

Suggested change

// clang-format off

nans_present_bvecs = 1;

github-actions · 2025-09-03T12:35:39Z

cpp/core/dwi/gradient.cpp

-    bvals(0, n) = grad(n, 3);
+  Axes::permutations_type order;
+  const auto adjusted_transform = File::NIfTI::adjust_transform(header, order);
+  Eigen::MatrixXd bvecs = adjusted_transform.inverse().linear() * grad.leftCols<3>().transpose();


warning: implicit conversion 'Scalar' (aka 'double') -> bool [readability-implicit-bool-conversion]

Suggested change

Eigen::MatrixXd bvecs = adjusted_transform.inverse().linear() * grad.leftCols<3>().transpose();

if (bvecs.row(n).squaredNorm() > 0.0 && (bvals[n] != 0.0) && bvals[n] <= bzero_threshold()) {

github-actions · 2025-09-03T12:35:39Z

cpp/core/dwi/gradient.cpp


  // bvecs format actually assumes a LHS coordinate system even if image is
-  // stored using RHS - x axis is flipped to make linear 3x3 part of
+  // stored using RHS; first axis is flipped to make linear 3x3 part of


warning: implicit conversion 'size_t' (aka 'unsigned long') -> bool [readability-implicit-bool-conversion]

Suggested change

// stored using RHS; first axis is flipped to make linear 3x3 part of

if (bval_zeroed_count != 0u) {

github-actions · 2025-09-03T12:35:40Z

cpp/core/metadata/slice_encoding.cpp

+    INFO("No need to transform slice encoding information for NIfTI image write:"
+         " image is already RAS");
+    return;
+  }


warning: narrowing conversion from 'size_t' (aka 'unsigned long') to signed type 'Index' (aka 'long') is implementation-defined [bugprone-narrowing-conversions]

new_dir[axis] = shuffle.flips[axis] ? -orig_dir[shuffle.permutations[axis]] : orig_dir[shuffle.permutations[axis]]; ^

github-actions · 2025-09-03T12:35:40Z

cpp/core/metadata/slice_encoding.cpp

+  if (one == "variable" || two == "variable")
+    return "variable";
+  std::vector<std::string> one_split = split(one, ",");
+  std::vector<std::string> two_split = split(two, ",");


warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]

Suggested change

std::vector<std::string> two_split = split(two, ",");

default_type f_one;

default_type f_two;

github-actions · 2025-09-03T12:35:40Z

cpp/core/metadata/slice_encoding.cpp

+  if (one == "variable" || two == "variable")
+    return "variable";
+  std::vector<std::string> one_split = split(one, ",");
+  std::vector<std::string> two_split = split(two, ",");


warning: variable 'f_one' is not initialized [cppcoreguidelines-init-variables]

default_type f_one, f_two; ^

this fix will not be applied because it overlaps with another fix

github-actions · 2025-09-03T12:35:40Z

cpp/core/metadata/slice_encoding.cpp

+  if (one == "variable" || two == "variable")
+    return "variable";
+  std::vector<std::string> one_split = split(one, ",");
+  std::vector<std::string> two_split = split(two, ",");


warning: variable 'f_two' is not initialized [cppcoreguidelines-init-variables]

default_type f_one, f_two; ^

this fix will not be applied because it overlaps with another fix

github-actions

clang-tidy made some suggestions

github-actions · 2025-09-03T13:10:48Z

cpp/core/dwi/gradient.cpp

+      if (grad.block<1, 3>(n, 0).squaredNorm() > 0.0)
+        // clang-format off
+        throw Exception("Corrupt content in bvecs/bvals data"
+                        " (" + bvecs_path + " & " + bvals_path + ")"


warning: string concatenation results in allocation of unnecessary temporary strings; consider using 'operator+=' or 'string::append()' instead [performance-inefficient-string-concatenation]

" (" + bvecs_path + " & " + bvals_path + ")" ^

github-actions · 2025-09-03T13:10:48Z

cpp/core/dwi/gradient.cpp

+      if (grad(n, 3) > 0.0)
+        // clang-format off
+        throw Exception("Corrupt content in bvecs/bvals data"
+                        " (" + bvecs_path + " & " + bvals_path + ")"


warning: string concatenation results in allocation of unnecessary temporary strings; consider using 'operator+=' or 'string::append()' instead [performance-inefficient-string-concatenation]

" (" + bvecs_path + " & " + bvals_path + ")" ^

Lestropie and others added 30 commits December 4, 2021 10:19

5ttcheck: Fix terminal message RE: -voxels option

b855539

Fix for mrview / singularity / NVIDIA

e1c372d

Added file / changes missing from ddca256

ba633f8

fsl.check_first(): Second attempt at refinement

0703abe

If possible, use fsl_sub command to halt execution until all jobs have completed. Result of discussion in #2597. Addresses #2595. Replicates some contents of bd3f19e.

fsl.check_first() fixes

18e5910

fsl.check_first(): Use dummy fsl_sub job to detect completion

f8477aa

fsl.check_first(): pylint fixes

7a661ad

fod2fixel: Fix -fmls_lobe_merge_ratio

039277f

Mismatch between option string as loaded in usage() and the get_options() call results in user-specified option being ignored. Error introduced in b435e58 as part of #1932.

Docs: Fix missing boolean types in full usage

5326435

mrinfo: Update author email

ba177bd

for_each: Add example usage demonstrating shell operator escape

692ee90

configure: strip FSL from PATH

fb1ce4f

DICOM: add null imageIO handler to allow parsing of DICOM data even w…

30bf591

…ith unsupported transfer syntax This at least allow mrinfo to run. Any operation that attempts to access the voxel data will of course fail.

Fix indentation in core/file/dicom/mapper.cpp

aef44bb

Co-authored-by: Robert Smith <[email protected]>

Reslice adapter: allow use with complex numbers

f97d3ca

DICOM: Improve documentation RE unsupported transfer syntaxes

57c0579

Merge remote-tracking branch 'origin/master' into allow_DICOM_header_…

4e6c6cf

…read_with_unsupported_transfer_syntax

mrgrid: Fix help for -as option

c7e5de4

Whitespace present in name of associated compulsory argument erroneously implies that there are two input arguments expected.

Merge branch 'master' into allow_DICOM_header_read_with_unsupported_t…

7f1d7ea

…ransfer_syntax

Add nightly checks with sanitizers

f55a1ed

Merge pull request #2785 from MRtrix3/reslice_complex_alternative

88c9367

Adapter::Reslice: Alternative resolution of complex summation

Merge branch 'master' into fix_reslice_adapter_for_complex

45871d5

Change schedule to weekly

4e87c73

Change sanitizers workflow name

1ca89c9

Merge pull request #2768 from MRtrix3/fix_reslice_adapter_for_complex

c52504b

Reslice adapter: allow use with complex numbers

This comment was marked as outdated.

Sign in to view

Merge commit 'c4b94c437a7955d25265f46ace48afd4ad2d5f66' into 305_to_dev

5253723

This comment was marked as outdated.

Sign in to view

Lestropie mentioned this pull request Aug 26, 2025

run.command(): Fix operation of -continue #3153

Merged

Lestropie force-pushed the dev branch from 70031c3 to f11ad0d Compare August 26, 2025 07:47

Lestropie force-pushed the 305_to_dev branch from 5253723 to ca1c1dc Compare August 26, 2025 07:47

Lestropie force-pushed the dev branch from f11ad0d to 70031c3 Compare August 26, 2025 07:51

Lestropie force-pushed the 305_to_dev branch from ca1c1dc to 5253723 Compare August 26, 2025 07:51

This comment was marked as outdated.

Sign in to view

Lestropie force-pushed the dev branch from 70031c3 to 6bf4cec Compare August 26, 2025 08:11

This was referenced Aug 27, 2025

Terminal fixes on dev #3137

Merged

Implement Pydra code-generators #2665

Open

Reversion of erroneous force pushes & branch stocktake #3167

Open

Merge branch 'dev' into 305_to_dev

fb51e46

This comment was marked as outdated.

Sign in to view

Lestropie and others added 5 commits September 3, 2025 19:58

DICOM: ignore image positioning tags if nested within Philips private…

c18f29a

… sequence Re-application of f148613 from #3047 as erroneously omitted from 1ebdb39 as part of #3101.

Update copyright headers #3101

30b5d7c

Add local disables of clang-format

73ed93d

Some of the attempted manual manipulation of string multi-line formatting either did not work as intended, or yielded a string overflow at compile time for certain compilers.

Update documentation to reflect tag 3.0.5

e61f3bd

github-actions bot reviewed Sep 3, 2025

View reviewed changes

Lestropie marked this pull request as ready for review September 3, 2025 13:47

Lestropie merged commit d3de24a into dev Sep 4, 2025
5 of 6 checks passed

Lestropie deleted the 305_to_dev branch September 4, 2025 00:01

Lestropie mentioned this pull request Sep 4, 2025

3.0.8 to dev #3170

Merged

	Eigen::MatrixXd grad(bvecs.cols(), 4);
	ssize_t nans_present_bvecs = 0;

	grad.leftCols<3>().transpose() = header.realignment().orig_transform().linear() * bvecs;
	ssize_t nans_present_bvals = 0;

	Eigen::MatrixXd bvecs = adjusted_transform.inverse().linear() * grad.leftCols<3>().transpose();
	if (bvecs.row(n).squaredNorm() > 0.0 && (bvals[n] != 0.0) && bvals[n] <= bzero_threshold()) {

	// stored using RHS; first axis is flipped to make linear 3x3 part of
	if (bval_zeroed_count != 0u) {

	std::vector<std::string> two_split = split(two, ",");
	default_type f_one;
	default_type f_two;

Merge 3.0.5 into dev #3101

Merge 3.0.5 into dev #3101

Uh oh!

Conversation

Lestropie commented May 21, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Lestropie commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Lestropie commented Sep 2, 2025 •

edited

Loading