tcksample: Permit 4D input image by Lestropie · Pull Request #2451 · MRtrix3/mrtrix3

Lestropie · 2022-03-12T23:16:50Z

Implemented for a specific project; listing here as draft for attribution to 3.1.0 milestone so that it doesn't get forgotten.

If some per-streamline statistic is to be computed, then it is possible to provide as input multiple images to be sampled from as a 4D volume series; the command uotput will then be a 2D matrix (one row per streamline, one column per metric) rather than a 1D vector (one scalar per streamline).

Lestropie · 2024-02-14T10:43:26Z

Add test data & CI test

Implemented class inheritance structure such that code branches based on the fundamental contrast being sampled does not need to occur for each streamline.

tcksample: Ability to sample SH amplitudes

Conflicts: cmd/tcksample.cpp

Lestropie · 2025-09-16T09:25:49Z

On merge, update dev branch and delete feature branch on test_data repo

github-actions

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 25 out of 40. Check the log or trigger a new build to see more.

github-actions · 2025-09-16T09:50:55Z

cpp/cmd/tcksample.cpp

 };

-template <class Interp> class SamplerNonPrecise {
+class SamplerBase {


warning: class 'SamplerBase' defines a non-default destructor and a copy constructor but does not define a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

class SamplerBase { ^

github-actions · 2025-09-16T09:50:55Z

cpp/cmd/tcksample.cpp

+  const stat_tck _statistic;
+};

+template <class Interp> class SamplerNonPreciseBase : public SamplerBase {


warning: class 'SamplerNonPreciseBase' defines a default destructor and a copy constructor but does not define a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

template <class Interp> class SamplerNonPreciseBase : public SamplerBase { ^

github-actions · 2025-09-16T09:50:55Z

cpp/cmd/tcksample.cpp

+  SamplerNonPreciseBase(Image<value_type> &image, const contrast_type contrast, const stat_tck statistic)
+      : BaseType(contrast, statistic), interp(image) {}
+  SamplerNonPreciseBase(const SamplerNonPreciseBase &that) = default;
+  virtual ~SamplerNonPreciseBase() = default;


warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [cppcoreguidelines-explicit-virtual-functions]

Suggested change

virtual ~SamplerNonPreciseBase() = default;

~SamplerNonPreciseBase() override = default;

github-actions · 2025-09-16T09:50:56Z

cpp/cmd/tcksample.cpp

  }

+protected:
+  Interp interp;


warning: member variable 'interp' has protected visibility [cppcoreguidelines-non-private-member-variables-in-classes]

Interp interp; ^

github-actions · 2025-09-16T09:50:56Z

cpp/cmd/tcksample.cpp

 };

-class SamplerPrecise {
+class SamplerPreciseBase : public SamplerBase {


warning: class 'SamplerPreciseBase' defines a default destructor and a copy constructor but does not define a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

class SamplerPreciseBase : public SamplerBase { ^

github-actions · 2025-09-16T09:50:58Z

cpp/cmd/tcksample.cpp

-      : ReceiverBase(num_tracks) {
+  using InputType = ManyPerStreamline;
+  Receiver_ManyPerStreamline(const size_t num_tracks, const size_t num_metrics, const std::string &path)
+      : ReceiverBase(num_tracks, false, path), data(matrix_type::Zero(num_tracks, num_metrics)) {}


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

: ReceiverBase(num_tracks, false, path), data(matrix_type::Zero(num_tracks, num_metrics)) {} ^

github-actions · 2025-09-16T09:50:58Z

cpp/cmd/tcksample.cpp

+  Receiver_ManyPerStreamline(const size_t num_tracks, const size_t num_metrics, const std::string &path)
+      : ReceiverBase(num_tracks, false, path), data(matrix_type::Zero(num_tracks, num_metrics)) {}
+  Receiver_ManyPerStreamline(const Receiver_ManyPerStreamline &) = delete;
+  ~Receiver_ManyPerStreamline() { File::Matrix::save_matrix(data, path); }


warning: annotate this function with 'override' or (rarely) 'final' [cppcoreguidelines-explicit-virtual-functions]

Suggested change

~Receiver_ManyPerStreamline() { File::Matrix::save_matrix(data, path); }

~Receiver_ManyPerStreamline() override { File::Matrix::save_matrix(data, path); }

github-actions · 2025-09-16T09:50:58Z

cpp/cmd/tcksample.cpp

+  bool operator()(InputType &in) {
+    // TODO Chance that this will be prohibitively slow if count is not indicated in track file header
+    if (in.index >= size_t(data.rows()))
+      data.conservativeResizeLike(matrix_type::Zero(in.index + 1, data.cols()));


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

data.conservativeResizeLike(matrix_type::Zero(in.index + 1, data.cols())); ^

github-actions · 2025-09-16T09:50:58Z

cpp/cmd/tcksample.cpp

+    // TODO Chance that this will be prohibitively slow if count is not indicated in track file header
+    if (in.index >= size_t(data.rows()))
+      data.conservativeResizeLike(matrix_type::Zero(in.index + 1, data.cols()));
+    data.row(in.index) = in.values;


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

data.row(in.index) = in.values; ^

github-actions · 2025-09-16T09:50:58Z

cpp/cmd/tcksample.cpp

+  matrix_type data;
+};
+
+class Receiver_PerVertex : public ReceiverBase {


warning: class 'Receiver_PerVertex' defines a copy constructor but does not define a destructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

class Receiver_PerVertex : public ReceiverBase { ^

Lestropie · 2025-09-16T10:37:45Z

Anyone with access to a Mac willing to help me look into the tcksample_inputsh_precise test? The CI is reporting a discrepancy of nearly 10%, which is rather extreme. I'm guessing that imprecisions with respect to the streamline-to-voxel mapping algorithm and the SH sampling are somehow stacking, but I wouldn't expect an error of that magnitude.

Add this at `cpp/cmd/tcksample.cpp:575:

    if (tck.get_index() == 112) {
      std::cerr << "Troublesome streamline 112:\n";
      std::cerr << "Intersections:\n";
      for (const auto &i : intersections)
        std::cerr << "  " << i.transpose() << ": " << i.get_length() << "\n";
      std::cerr << "Sampled data:\n";
      for (const auto &i : data)
        std::cerr << "  " << i.length << ": " << i.value << "\n";
      std::cerr << "Final result: " << out.value << "\n";
    }

Here's what my Linux box produces"

Troublesome streamline 112:
Intersections:
  0 4 3: 0.777862
  0 5 3: 1.80769
  0 4 4: 2.70528
  1 4 4: 0.363316
  2 3 5: 1.16057
  1 4 5: 3.05493
  2 4 5: 0.0803039
Sampled data:
  0.777862: -0.0445911
  1.80769: 0.360195
  2.70528: 0.129052
  0.363316: 0.324144
  1.16057: 0.239748
  3.05493: 0.429606
  0.0803039: 0.0256022
Final result: 0.26895

Lestropie added 2 commits March 13, 2022 09:51

tcksample: Fix compilation RE: NaN usage

012eb5a

Lestropie added the enhancement label Mar 12, 2022

Lestropie added this to the 3.1.0 updates milestone Mar 12, 2022

Lestropie self-assigned this Mar 12, 2022

Lestropie mentioned this pull request Mar 12, 2022

tcksample 4D image input sina-mansour/UKB-connectomics#29

Closed

Lestropie mentioned this pull request Jul 15, 2024

tcksample: Sample FOD amplitudes #2942

Closed

3 tasks

tcksample: Ability to sample SH amplitudes

4fd6f45

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

Lestropie force-pushed the tcksample_4d branch from 012eb5a to 5deef15 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 tcksample_4d branch from 5deef15 to 012eb5a Compare August 26, 2025 07:51

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

tcksample: Refactoring of SH handling

7c8631f

Implemented class inheritance structure such that code branches based on the fundamental contrast being sampled does not need to occur for each streamline.

Lestropie mentioned this pull request Sep 16, 2025

tcksample: Ability to sample SH amplitudes #3028

Merged

2 tasks

Lestropie added 5 commits September 16, 2025 13:41

Merge pull request #3028 from MRtrix3/tcksample_fod

47cefb7

tcksample: Ability to sample SH amplitudes

Merge remote-tracking branch 'origin/dev' into tcksample_4d

8d4368c

Conflicts: cmd/tcksample.cpp

tcksample: Fix compilation after merge to post-cmake dev

766a466

tcksample: Test data and tests for implementation updates

791bced

tcksample: Remove discarded portions of updated implementation

df0947b

Lestropie marked this pull request as ready for review September 16, 2025 09:21

tcksample: Re-generate copyright, docs

8d2860c

This comment was marked as outdated.

Sign in to view

Lestropie added 2 commits September 16, 2025 19:41

tcksample: Tweaks to new implementation by clang-tidy

8595088

tcksample: Relax some testing tolerances for MacOS CI

8d8c1fd

github-actions bot reviewed Sep 16, 2025

View reviewed changes

	virtual ~SamplerNonPreciseBase() = default;
	~SamplerNonPreciseBase() override = default;

	~Receiver_ManyPerStreamline() { File::Matrix::save_matrix(data, path); }
	~Receiver_ManyPerStreamline() override { File::Matrix::save_matrix(data, path); }

Conversation

Lestropie commented Mar 12, 2022

Uh oh!

Lestropie commented Feb 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lestropie commented Sep 16, 2025

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 16, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Lestropie commented Sep 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Lestropie commented Feb 14, 2024 •

edited

Loading