Skip to content

New command maskdistance#3166

Draft
Lestropie wants to merge 1 commit intodevfrom
maskdistance
Draft

New command maskdistance#3166
Lestropie wants to merge 1 commit intodevfrom
maskdistance

Conversation

@Lestropie
Copy link
Copy Markdown
Member

Listing so as to not have dangling branches. Would need to revisit to remind myself of how mature or generalised this was.

This was intended to do something akin to tractometry / tckresample | tcksample.
But instead of generating values along each streamline, it would instead use the streamlines trajectories to assign to each voxel a value between 0 and 1 encoding its relative distance along the path between two ROIs. That could then be used to group voxels into bins, or indeed to generate a scatterplot along the length; either way, each image sample would contribute to the output data only once.

@Lestropie Lestropie self-assigned this Aug 29, 2025
Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

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

Comment thread cmd/maskdistance.cpp

value_type angular_threshold_dp()
{
return std::cos (45.0 * (Math::pi / 180.0));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: narrowing conversion from 'double' to 'value_type' (aka 'float') [bugprone-narrowing-conversions]

  return std::cos (45.0 * (Math::pi / 180.0));
         ^

Comment thread cmd/maskdistance.cpp

class TckVisitation;

class TargetBase
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

class TargetBase
      ^

Comment thread cmd/maskdistance.cpp
virtual const Header& header() const = 0;
virtual size_t nfixels() const = 0;

virtual void map (const Eigen::Vector3f& pos, const Eigen::Vector3f& dir, const value_type dist, TckVisitation& tck) const = 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter 'dist' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change
virtual void map (const Eigen::Vector3f& pos, const Eigen::Vector3f& dir, const value_type dist, TckVisitation& tck) const = 0;
virtual void map (const Eigen::Vector3f& pos, const Eigen::Vector3f& dir, value_type dist, TckVisitation& tck) const = 0;

Comment thread cmd/maskdistance.cpp
void add (const size_t fixel_index, const value_type distance)
{
assert (fixel_index < fixel_mindist_sum.size());
fixel_mindist_sum[fixel_index] += distance;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

      fixel_mindist_sum[fixel_index] += distance;
                        ^

Comment thread cmd/maskdistance.cpp
{
assert (fixel_index < fixel_mindist_sum.size());
fixel_mindist_sum[fixel_index] += distance;
fixel_vertexcount[fixel_index]++;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

      fixel_vertexcount[fixel_index]++;
                        ^

Comment thread cmd/maskdistance.cpp
}
// All values now finite; need to start accumulating distance after the first zero is encountered
// Also only over-write non-zero values if the distance in this direction is smaller
distance = result[num_vertices-1] ? NaN : 0.0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: narrowing conversion from 'value_type' (aka 'float') to 'bool' [bugprone-narrowing-conversions]

  distance = result[num_vertices-1] ? NaN : 0.0;
             ^

Comment thread cmd/maskdistance.cpp
// All values now finite; need to start accumulating distance after the first zero is encountered
// Also only over-write non-zero values if the distance in this direction is smaller
distance = result[num_vertices-1] ? NaN : 0.0;
for (ssize_t v = num_vertices-2; v >= 0; --v) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  for (ssize_t v = num_vertices-2; v >= 0; --v) {
                   ^

Comment thread cmd/maskdistance.cpp
target_t target_type (target_t::UNDEFINED);
if (Path::has_suffix (output_path, "tsf")) {

target_type = target_t::TSF;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: Value stored to 'target_type' is never read [clang-analyzer-deadcode.DeadStores]

    target_type = target_t::TSF;
    ^
Additional context

cmd/maskdistance.cpp:402: Value stored to 'target_type' is never read

    target_type = target_t::TSF;
    ^

Comment thread cmd/maskdistance.cpp
while (reader (tck_in)) {
DWI::Tractography::TrackScalar<> vertex_roi_dist = vertex_distances (tck_in, roi);
// Can't use non-finite values in track scalar file due to their use as delimiters
if (!vertex_roi_dist.size())
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

      if (!vertex_roi_dist.size())
           ^

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

Comment thread cmd/maskdistance.cpp
while (reader (tck_in)) {
DWI::Tractography::TrackScalar<> vertex_roi_dist = vertex_distances (tck_in, roi);
// Can't use non-finite values in track scalar file due to their use as delimiters
if (!vertex_roi_dist.size())
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]

Suggested change
if (!vertex_roi_dist.size())
if (vertex_roi_dist.empty())
Additional context

/usr/include/c++/13/bits/stl_vector.h:1087: method 'vector'::empty() defined here

      empty() const _GLIBCXX_NOEXCEPT
      ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant