From 8801b88092868c159741b1c575971e33cc2a115f Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Tue, 2 Jun 2026 01:04:42 +1000 Subject: [PATCH 1/5] Convert command-line choice options to enum-based type_choice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the std::vector-based Argument::type_choice overload with the enum template type_choice(), converting every command-line choice option to operate on a strongly-typed enumeration parsed via MR::Enum::from_name / App::get_option_choice. The string vectors that previously defined each option's choices are removed in favour of the backing enums. Enumerators that are meaningful internally but must not be user-selectable (such as tck_stat_t's ENDS_CORR) are excluded from the presented choices by a magic_enum customize::enum_name specialisation, which also causes such values to be rejected when supplied, obviating any parallel CLI-only enumeration. Command-line help text and accepted values are preserved throughout. Session prompts: 1. > In class MR::Argument (cpp/core/cmdline_option.h), remove member > function type_choice(const std::vector &). For all > resulting compilation failures, substitute with template > type_choice(), defining an appropriate enum if one does not > exist, and modify all parsing of that option to operate on the enum. > Remove any std::vector used to define the choices, and > re-confirm successful compilation. 2. > 1. Where get_option_value() or a ternary was used to access an enum > choice or set a default, use get_option_choice() instead. > 2. Where the CLI options are only a subset of an enum's states, find > code detrimentally affected (eg. regression to integers); comment > on whether a std::variant over two enumerations would be beneficial. > 3. Make DataType::Identifier's 8-bit values equal each datatype's > internal DataType encoding. 3. > 1. Augment type_choice() with an optional > std::optional> of elements not to be > presented at the command-line. > 2. Use it to obviate tck_stat_cli_t. > 3. Revert task 3 above, keeping DataType::Identifier sequentially > indexed in the original order so help strings are unaltered. 4. > For points 1 and 2, revisit enum subsets using magic_enum features > rather than a specified exclusion list — better use of the library. > Report how the corresponding code may appear. Generated-by: Claude Opus 4.8 (1M context) --- cpp/cmd/mraverageheader.cpp | 9 +-- cpp/cmd/mrcolour.cpp | 15 ++-- cpp/cmd/mrgrid.cpp | 9 +-- cpp/cmd/mrmetric.cpp | 7 +- cpp/cmd/mrregister.cpp | 77 ++++--------------- cpp/cmd/mrstats.cpp | 5 +- cpp/cmd/mrtransform.cpp | 17 ++-- cpp/cmd/tck2connectome.cpp | 7 +- cpp/cmd/tckdfc.cpp | 8 +- cpp/cmd/tckmap.cpp | 29 ++++--- cpp/core/cmdline_option.h | 22 ++---- cpp/core/colourmap.h | 9 +++ cpp/core/datatype.cpp | 12 +-- cpp/core/datatype.h | 47 ++++++++++- .../dwi/tractography/connectome/matrix.cpp | 7 +- cpp/core/dwi/tractography/connectome/matrix.h | 1 - .../dwi/tractography/mapping/twi_stats.cpp | 17 ---- cpp/core/dwi/tractography/mapping/twi_stats.h | 23 +++++- cpp/core/interp/interp.h | 4 - cpp/core/math/average_space.cpp | 3 - cpp/core/math/average_space.h | 2 - cpp/core/registration/linear.cpp | 76 +++++++----------- cpp/core/registration/linear.h | 12 ++- cpp/core/stats.cpp | 6 +- cpp/core/stats.h | 36 +++++---- .../mrview/tool/tractography/tractography.cpp | 24 +++--- .../mrview/tool/tractography/tractography.h | 2 - testing/data/cpp_cli/full_usage.txt | 4 +- testing/tools/testing_cpp_cli.cpp | 6 +- 29 files changed, 223 insertions(+), 273 deletions(-) diff --git a/cpp/cmd/mraverageheader.cpp b/cpp/cmd/mraverageheader.cpp index 40bd1341e9..499f567bbf 100644 --- a/cpp/cmd/mraverageheader.cpp +++ b/cpp/cmd/mraverageheader.cpp @@ -17,6 +17,7 @@ #include "algo/loop.h" #include "command.h" #include "debug.h" +#include "enum.h" #include "image.h" #include "interp/nearest.h" #include "math/average_space.h" @@ -62,9 +63,9 @@ void usage() { "Method for determination of voxel spacings based on" " the set of input images and the average header axes" " (see Description)." - " Valid options are: " + join(avgspace_voxspacing_choices, ",") + ";" + " Valid options are: " + MR::Enum::join(",") + ";" " default = " + SPACING_DEFAULT_STRING) - + Argument("type").type_choice(avgspace_voxspacing_choices) + + Argument("type").type_choice() + Option ("fill", "set the intensity in the first volume of the average space to 1") + DataType::options(); @@ -80,9 +81,7 @@ void run() { const default_type p = get_option_value("padding", PADDING_DEFAULT); auto padding = Eigen::Matrix(p, p, p, 1.0); INFO("padding in template voxels: " + str(padding.transpose().head<3>())); - auto opt = get_options("spacing"); - const avgspace_voxspacing_t spacing = - opt.empty() ? SPACING_DEFAULT_VALUE : static_cast(static_cast(opt[0][0])); + const avgspace_voxspacing_t spacing = get_option_choice("spacing", SPACING_DEFAULT_VALUE); const bool fill = !get_options("fill").empty(); std::vector
headers_in; diff --git a/cpp/cmd/mrcolour.cpp b/cpp/cmd/mrcolour.cpp index dadbb7ad96..37a315c2e2 100644 --- a/cpp/cmd/mrcolour.cpp +++ b/cpp/cmd/mrcolour.cpp @@ -16,6 +16,7 @@ #include "command.h" #include "datatype.h" +#include "enum.h" #include "header.h" #include "image.h" #include "mrtrix.h" @@ -29,16 +30,9 @@ using namespace MR; using namespace App; -std::vector colourmap_choices; - // clang-format off void usage() { - for (const auto& entry : ColourMap::maps) { - if (entry.name != "Complex") - colourmap_choices.push_back(lowercase(entry.name)); - } - AUTHOR = "Robert E. Smith (robert.smith@florey.edu.au)"; SYNOPSIS = "Apply a colour map to an image"; @@ -60,7 +54,7 @@ void usage() { ARGUMENTS + Argument ("input", "the input image").type_image_in() + Argument ("map", "the colourmap to apply;" - " choices are: " + join(colourmap_choices, ",")).type_choice (colourmap_choices) + " choices are: " + MR::Enum::join(",")).type_choice() + Argument ("output", "the output image").type_image_out(); OPTIONS @@ -79,7 +73,10 @@ void usage() { void run() { Header H_in = Header::open(argument[0]); - const ColourMap::Entry colourmap = ColourMap::maps[argument[1]]; + // Resolve the selected colour map by name rather than by integer index, + // so the ColourMap::Choice enumeration need not be kept in lock-step with the maps table. + const ColourMap::Entry colourmap = + ColourMap::maps[ColourMap::index(MR::Enum::name(MR::Enum::from_name(argument[1])))]; Eigen::Vector3d fixed_colour(Eigen::Vector3d::Constant(NaN)); if (colourmap.is_colour) { if (!(H_in.ndim() == 3 || (H_in.ndim() == 4 && H_in.size(3) == 1))) diff --git a/cpp/cmd/mrgrid.cpp b/cpp/cmd/mrgrid.cpp index ae34a7cb97..6dd09ac669 100644 --- a/cpp/cmd/mrgrid.cpp +++ b/cpp/cmd/mrgrid.cpp @@ -131,9 +131,9 @@ void usage() { + Argument ("factor").type_sequence_float() + Option ("interp", std::string("set the interpolation method to use when reslicing") - + " (choices: " + join(MR::Interp::interp_choices, ", ") + ";" - " default: " + MR::Interp::interp_choices[static_cast(default_interp)] + ").") - + Argument ("method").type_choice (MR::Interp::interp_choices) + + " (choices: " + MR::Enum::join() + ";" + " default: " + MR::Enum::lowercase_name(default_interp) + ").") + + Argument ("method").type_choice() + Option ("oversample", "set the amount of over-sampling (in the target space) to perform when regridding." @@ -221,8 +221,7 @@ void run() { regrid_filter.set_out_of_bounds_value(out_of_bounds_value); size_t resize_option_count = 0; size_t template_option_count = 0; - const MR::Interp::interp_type interp = - MR::Interp::interp_type(get_option_value("interp", static_cast(default_interp))); + const MR::Interp::interp_type interp = get_option_choice("interp", default_interp); // over-sampling std::vector oversample = Adapter::AutoOverSample; diff --git a/cpp/cmd/mrmetric.cpp b/cpp/cmd/mrmetric.cpp index 2bfde76baa..271265a9ea 100644 --- a/cpp/cmd/mrmetric.cpp +++ b/cpp/cmd/mrmetric.cpp @@ -200,8 +200,8 @@ void usage() { + Option ("interp", std::string("set the interpolation method to use when reslicing") + " (choices: nearest, linear, cubic, sinc." - " Default: " + MR::Interp::interp_choices[static_cast(default_interp)] + ").") - + Argument ("method").type_choice(MR::Interp::interp_choices) + " Default: " + MR::Enum::lowercase_name(default_interp) + ").") + + Argument ("method").type_choice() + Option ("metric", "define the dissimilarity metric used to calculate the cost." @@ -231,8 +231,7 @@ using MaskType = Image; void run() { const space_t space = get_option_choice("space", default_space); - const MR::Interp::interp_type interp = - MR::Interp::interp_type(get_option_value("interp", static_cast(default_interp))); + const MR::Interp::interp_type interp = get_option_choice("interp", default_interp); MetricType metric_type = MetricType::MeanSquared; const MetricChoice metric_choice = get_option_choice("metric", MetricChoice::DIFF); diff --git a/cpp/cmd/mrregister.cpp b/cpp/cmd/mrregister.cpp index c4ac85dbfe..8943c74e8b 100644 --- a/cpp/cmd/mrregister.cpp +++ b/cpp/cmd/mrregister.cpp @@ -402,12 +402,14 @@ void run() { if (!opt.empty()) { if (init_rigid_matrix_set) throw Exception("options -rigid_init_matrix and -rigid_init_translation are mutually exclusive"); - Registration::set_init_translation_model_from_option(rigid_registration, static_cast(opt[0][0])); + Registration::set_init_translation_model_from_option( + rigid_registration, MR::Enum::from_name(opt[0][0])); } opt = get_options("rigid_init_rotation"); if (!opt.empty()) - Registration::set_init_rotation_model_from_option(rigid_registration, static_cast(opt[0][0])); + Registration::set_init_rotation_model_from_option(rigid_registration, + MR::Enum::from_name(opt[0][0])); opt = get_options("rigid_scale"); if (!opt.empty()) { @@ -430,20 +432,8 @@ void run() { rigid_registration.set_max_iter(parse_ints(opt[0][0])); } - opt = get_options("rigid_metric"); - Registration::LinearMetricType rigid_metric = Registration::Diff; - if (!opt.empty()) { - switch (static_cast(opt[0][0])) { - case 0: - rigid_metric = Registration::Diff; - break; - case 1: - rigid_metric = Registration::NCC; - break; - default: - break; - } - } + const Registration::LinearMetricType rigid_metric = + get_option_choice("rigid_metric", Registration::Diff); if (rigid_metric == Registration::NCC) throw Exception("TODO: cross correlation metric not yet implemented"); @@ -453,21 +443,7 @@ void run() { if (!opt.empty()) { if (rigid_metric != Registration::Diff) throw Exception("rigid_metric.diff.estimator set but cost function is not diff."); - switch (static_cast(opt[0][0])) { - case 0: - rigid_estimator = Registration::L1; - break; - case 1: - rigid_estimator = Registration::L2; - break; - case 2: - rigid_estimator = Registration::LP; - break; - case 3: - rigid_estimator = Registration::None; - default: - assert(false); - } + rigid_estimator = MR::Enum::from_name(opt[0][0]); } opt = get_options("rigid_lmax"); @@ -534,14 +510,16 @@ void run() { if (!opt.empty()) { if (init_affine_matrix_set) throw Exception("options -affine_init_matrix and -affine_init_translation are mutually exclusive"); - Registration::set_init_translation_model_from_option(affine_registration, static_cast(opt[0][0])); + Registration::set_init_translation_model_from_option( + affine_registration, MR::Enum::from_name(opt[0][0])); } opt = get_options("affine_init_rotation"); if (!opt.empty()) { if (init_affine_matrix_set) throw Exception("options -affine_init_matrix and -affine_init_rotation are mutually exclusive"); - Registration::set_init_rotation_model_from_option(affine_registration, static_cast(opt[0][0])); + Registration::set_init_rotation_model_from_option(affine_registration, + MR::Enum::from_name(opt[0][0])); } opt = get_options("affine_scale"); @@ -558,20 +536,8 @@ void run() { affine_registration.set_loop_density(parse_floats(opt[0][0])); } - opt = get_options("affine_metric"); - Registration::LinearMetricType affine_metric = Registration::Diff; - if (!opt.empty()) { - switch (static_cast(opt[0][0])) { - case 0: - affine_metric = Registration::Diff; - break; - case 1: - affine_metric = Registration::NCC; - break; - default: - break; - } - } + const Registration::LinearMetricType affine_metric = + get_option_choice("affine_metric", Registration::Diff); if (affine_metric == Registration::NCC) throw Exception("TODO cross correlation metric not yet implemented"); @@ -581,22 +547,7 @@ void run() { if (!opt.empty()) { if (affine_metric != Registration::Diff) throw Exception("affine_metric.diff.estimator set but cost function is not diff."); - switch (static_cast(opt[0][0])) { - case 0: - affine_estimator = Registration::L1; - break; - case 1: - affine_estimator = Registration::L2; - break; - case 2: - affine_estimator = Registration::LP; - break; - case 3: - affine_estimator = Registration::None; - break; - default: - assert(false); - } + affine_estimator = MR::Enum::from_name(opt[0][0]); } opt = get_options("affine_niter"); diff --git a/cpp/cmd/mrstats.cpp b/cpp/cmd/mrstats.cpp index 03c7ccc899..bbb2574215 100644 --- a/cpp/cmd/mrstats.cpp +++ b/cpp/cmd/mrstats.cpp @@ -18,6 +18,7 @@ #include "command.h" #include "datatype.h" +#include "enum.h" #include "image.h" #include "image_helpers.h" #include "memory.h" @@ -108,10 +109,10 @@ void run() { check_dimensions(mask, header, 0, 3); } - std::vector fields; + std::vector fields; opt = get_options("output"); for (size_t n = 0; n < opt.size(); ++n) - fields.push_back(opt[n][0]); + fields.push_back(MR::Enum::from_name(opt[n][0])); if (App::log_level && fields.empty()) Stats::print_header(is_complex); diff --git a/cpp/cmd/mrtransform.cpp b/cpp/cmd/mrtransform.cpp index bd4235a4f3..9c24269aa8 100644 --- a/cpp/cmd/mrtransform.cpp +++ b/cpp/cmd/mrtransform.cpp @@ -26,6 +26,7 @@ #include "dwi/directions/predefined.h" #include "dwi/directions/validate.h" #include "dwi/gradient.h" +#include "enum.h" #include "file/matrix.h" #include "file/nifti_utils.h" #include "filter/reslice.h" @@ -156,9 +157,9 @@ void usage() { + Option ("interp", std::string("set the interpolation method to use when reslicing") - + " (choices: " + join(MR::Interp::interp_choices, ", ") + ";" - + " default: " + MR::Interp::interp_choices[static_cast(default_interp)] + ").") - + Argument ("method").type_choice(MR::Interp::interp_choices) + + " (choices: " + MR::Enum::join() + ";" + + " default: " + MR::Enum::lowercase_name(default_interp) + ").") + + Argument ("method").type_choice() + Option ("oversample", "set the amount of over-sampling (in the target space) to perform when regridding." @@ -584,13 +585,9 @@ void run() { } // Interpolator - MR::Interp::interp_type interp = default_interp; - opt = get_options("interp"); - if (!opt.empty()) { - interp = MR::Interp::interp_type(static_cast(opt[0][0])); - if (!warp && !template_header) - WARN("interpolator choice ignored since the input image will not be regridded"); - } + const MR::Interp::interp_type interp = get_option_choice("interp", default_interp); + if (!get_options("interp").empty() && !warp && !template_header) + WARN("interpolator choice ignored since the input image will not be regridded"); // over-sampling std::vector oversample = Adapter::AutoOverSample; diff --git a/cpp/cmd/tck2connectome.cpp b/cpp/cmd/tck2connectome.cpp index 13a0d6f734..4d4cf3d19f 100644 --- a/cpp/cmd/tck2connectome.cpp +++ b/cpp/cmd/tck2connectome.cpp @@ -17,6 +17,7 @@ #include #include "command.h" +#include "enum.h" #include "image.h" #include "thread_queue.h" #include "types.h" @@ -154,9 +155,7 @@ void execute(Image &node_image, const node_t max_node_index, const std:: Metric metric; Tractography::Connectome::setup_metric(metric, node_image); std::unique_ptr tck2nodes(load_assignment_mode(node_image)); - auto opt = get_options("stat_edge"); - const stat_edge statistic = - !opt.empty() ? stat_edge(static_cast(opt[0][0])) : stat_edge::SUM; + const stat_edge statistic = get_option_choice("stat_edge", stat_edge::SUM); // Prepare for reading the track data Tractography::Properties properties; @@ -191,7 +190,7 @@ void execute(Image &node_image, const node_t max_node_index, const std:: get_options("symmetric").size(), get_options("zero_diagonal").size()); - opt = get_options("out_assignments"); + auto opt = get_options("out_assignments"); if (!opt.empty()) connectome.write_assignments(opt[0][0]); } diff --git a/cpp/cmd/tckdfc.cpp b/cpp/cmd/tckdfc.cpp index 2c79d3c63d..3fd181124f 100644 --- a/cpp/cmd/tckdfc.cpp +++ b/cpp/cmd/tckdfc.cpp @@ -120,9 +120,9 @@ void usage () { "define the statistic for choosing the final voxel intensities" " for a given contrast type given the individual values" " from the tracks passing through each voxel;" - " options are: " + join(voxel_statistics, ", ") + + " options are: " + MR::Enum::join() + " (default: mean)") - + Argument ("type").type_choice(voxel_statistics) + + Argument ("type").type_choice() + OptionGroup ("Other options for affecting the streamline sampling & mapping behaviour") @@ -353,9 +353,7 @@ void run() { } } - opt = get_options("stat_vox"); - const vox_stat_t stat_vox = - !opt.empty() ? vox_stat_t(static_cast(opt[0][0])) : vox_stat_t::MEAN; + const vox_stat_t stat_vox = get_option_choice("stat_vox", vox_stat_t::MEAN); Header H_3D(header); H_3D.ndim() = 3; diff --git a/cpp/cmd/tckmap.cpp b/cpp/cmd/tckmap.cpp index 905d129953..a0328ee08a 100644 --- a/cpp/cmd/tckmap.cpp +++ b/cpp/cmd/tckmap.cpp @@ -17,6 +17,7 @@ #include #include "command.h" +#include "enum.h" #include "image.h" #include "memory.h" #include "progressbar.h" @@ -55,7 +56,7 @@ const OptionGroup OutputHeaderOption = OptionGroup ("Options for the header of t " or comma-separated list of 3 voxel dimensions.") + Argument ("size").type_sequence_float() + Option ("datatype", "specify output image data type.") - + Argument ("spec").type_choice(DataType::identifiers); + + Argument ("spec").type_choice(); const OptionGroup OutputDimOption = OptionGroup ("Options for the dimensionality of the output image") + Option ("dec", @@ -76,9 +77,9 @@ const OptionGroup OutputDimOption = OptionGroup ("Options for the dimensionality const OptionGroup TWIOption = OptionGroup ("Options for the TWI image contrast properties") + Option ("contrast", "define the desired form of contrast for the output image;" - " options are: " + join(contrasts, ", ") + + " options are: " + MR::Enum::join() + " (default: tdi)") - + Argument ("type").type_choice(contrasts) + + Argument ("type").type_choice() + Option ("image", "provide the scalar image map for generating images with 'scalar_map' / 'scalar_map_count' contrast," " or the spherical harmonics image for 'fod_amp' contrast") @@ -89,16 +90,16 @@ const OptionGroup TWIOption = OptionGroup ("Options for the TWI image contrast p + Option ("stat_vox", "define the statistic for choosing the final voxel intensities for a given contrast type" " given the individual values from the tracks passing through each voxel." - " Options are: " + join(voxel_statistics, ", ") + + " Options are: " + MR::Enum::join() + " (default: sum)") - + Argument ("type").type_choice(voxel_statistics) + + Argument ("type").type_choice() + Option ("stat_tck", "define the statistic for choosing the contribution to be made by each streamline" " as a function of the samples taken along their lengths." " Only has an effect for 'scalar_map', 'fod_amp' and 'curvature' contrast types." - " Options are: " + join(track_statistics, ", ") + + " Options are: " + MR::Enum::join() + " (default: mean)") - + Argument ("type").type_choice(track_statistics) + + Argument ("type").type_choice() + Option ("fwhm_tck", "when using gaussian-smoothed per-track statistic," " specify the desired full-width half-maximum of the Gaussian smoothing kernel" @@ -305,17 +306,13 @@ void run() { add_line(header.keyval()["comments"], "track-weighted image"); header.keyval()["tck_source"] = input_tracks_path.filename().string(); - opt = get_options("contrast"); - const contrast_t contrast = - !opt.empty() ? contrast_t(static_cast(opt[0][0])) : contrast_t::TDI; + const contrast_t contrast = get_option_choice("contrast", contrast_t::TDI); - opt = get_options("stat_vox"); - vox_stat_t stat_vox = - !opt.empty() ? vox_stat_t(static_cast(opt[0][0])) : vox_stat_t::SUM; + vox_stat_t stat_vox = get_option_choice("stat_vox", vox_stat_t::SUM); - opt = get_options("stat_tck"); - tck_stat_t stat_tck = - !opt.empty() ? tck_stat_t(static_cast(opt[0][0])) : tck_stat_t::MEAN; + // ENDS_CORR is excluded from tck_stat_t's magic_enum reflection (see twi_stats.h), + // so get_option_choice() (via from_name) rejects it as an unsupported value. + tck_stat_t stat_tck = get_option_choice("stat_tck", tck_stat_t::MEAN); float gaussian_fwhm_tck = 0.0; opt = get_options("fwhm_tck"); diff --git a/cpp/core/cmdline_option.h b/cpp/core/cmdline_option.h index 2724e63d54..227ffd2edf 100644 --- a/cpp/core/cmdline_option.h +++ b/cpp/core/cmdline_option.h @@ -242,23 +242,6 @@ class Argument { return *this; } - //! specifies that the argument should be selected from a predefined list - /*! The list of allowed values must be specified as a vector of strings. - * Here is an example usage: - * \code - * const std::vector mode_list = { "standard", "pedantic", "approx" }; - * - * ARGUMENTS - * + Argument ("mode", "the mode of operation") - * .type_choice (mode_list); - * \endcode - * \note Each string in the list must be supplied in \b lowercase. */ - Argument &type_choice(const std::vector &c) { - types.set(ArgTypeFlags::Choice); - choices = c; - return *this; - } - //! specifies that the argument should be selected from a predefined list of enum values /*! The list of allowed values is automatically generated from the enum type provided as template parameter. * Here is an example usage: @@ -269,6 +252,11 @@ class Argument { * + Argument ("mode", "the mode of operation") * .type_choice(); * \endcode + * Enumerators that are meaningful internally but should not be user-selectable can be + * omitted from the presented set by excluding them from the enum's magic_enum reflection + * (a magic_enum::customize::enum_name specialization returning invalid_tag, declared at the + * enum definition); such values are then absent from the choices here, and are rejected by + * MR::Enum::from_name() / App::get_option_choice() when supplied on the command-line. * \note Each enum value in the list must be supplied in \b lowercase. */ template Argument &type_choice() { static_assert(std::is_enum_v, "Template parameter must be an enum type"); diff --git a/cpp/core/colourmap.h b/cpp/core/colourmap.h index b085f6a84a..4674d31abc 100644 --- a/cpp/core/colourmap.h +++ b/cpp/core/colourmap.h @@ -53,6 +53,15 @@ class Entry { extern const std::vector maps; +//! enumeration of the colour maps that can be selected on the command-line +/*! The lowercase enumerator names define the set of valid choices for + * colour-map command-line options. Each enumerator name matches the \c name + * field of the corresponding entry in the \c maps table, so a selection can be + * resolved to a \c maps entry via index() without relying on the enumerator's + * underlying integer value. The "Complex" map is deliberately omitted, as it is + * not offered as a command-line selection. */ +enum class Choice { Gray, Hot, Cool, Jet, Inferno, Viridis, PET, Colour, RGB }; + inline size_t num() { return maps.size(); } inline size_t num_scalar() { diff --git a/cpp/core/datatype.cpp b/cpp/core/datatype.cpp index e4f8ec067e..c5e59f83d2 100644 --- a/cpp/core/datatype.cpp +++ b/cpp/core/datatype.cpp @@ -16,6 +16,7 @@ #include "datatype.h" #include "app.h" +#include "enum.h" #include namespace MR { @@ -68,13 +69,6 @@ constexpr uint8_t DataType::CFloat64LE; constexpr uint8_t DataType::CFloat64BE; constexpr uint8_t DataType::Native; -const std::vector DataType::identifiers = { - "float16", "float16le", "float16be", "float32", "float32le", "float32be", "float64", "float64le", - "float64be", "int64", "uint64", "int64le", "uint64le", "int64be", "uint64be", "int32", - "uint32", "int32le", "uint32le", "int32be", "uint32be", "int16", "uint16", "int16le", - "uint16le", "int16be", "uint16be", "cfloat16", "cfloat16le", "cfloat16be", "cfloat32", "cfloat32le", - "cfloat32be", "cfloat64", "cfloat64le", "cfloat64be", "int8", "uint8", "bit"}; - DataType DataType::parse(std::string_view spec) { std::string str(lowercase(spec)); @@ -255,8 +249,8 @@ App::OptionGroup DataType::options() { return OptionGroup("Data type options") + Option("datatype", "specify output image data type." " Valid choices are: " - + join(identifiers, ", ") + ".") - + Argument("spec").type_choice(identifiers); + + MR::Enum::join() + ".") + + Argument("spec").type_choice(); } // clang-format on diff --git a/cpp/core/datatype.h b/cpp/core/datatype.h index a5c769c192..752f3335a3 100644 --- a/cpp/core/datatype.h +++ b/cpp/core/datatype.h @@ -166,7 +166,52 @@ class DataType { #endif static App::OptionGroup options(); - static const std::vector identifiers; + //! enumeration of the data type specifiers accepted on the command-line + /*! The lowercase names of these enumerators define the set of valid choices + * for the "-datatype" command-line option; see DataType::options(). + * The enumerators are declared in the same order as the historical list of + * choice strings, so that the generated command-line help is unaltered. */ + enum class Identifier { + Float16, + Float16LE, + Float16BE, + Float32, + Float32LE, + Float32BE, + Float64, + Float64LE, + Float64BE, + Int64, + UInt64, + Int64LE, + UInt64LE, + Int64BE, + UInt64BE, + Int32, + UInt32, + Int32LE, + UInt32LE, + Int32BE, + UInt32BE, + Int16, + UInt16, + Int16LE, + UInt16LE, + Int16BE, + UInt16BE, + CFloat16, + CFloat16LE, + CFloat16BE, + CFloat32, + CFloat32LE, + CFloat32BE, + CFloat64, + CFloat64LE, + CFloat64BE, + Int8, + UInt8, + Bit + }; friend std::ostream &operator<<(std::ostream &stream, const DataType &dt) { stream << dt.specifier(); diff --git a/cpp/core/dwi/tractography/connectome/matrix.cpp b/cpp/core/dwi/tractography/connectome/matrix.cpp index b47ebe05fa..07220d62f0 100644 --- a/cpp/core/dwi/tractography/connectome/matrix.cpp +++ b/cpp/core/dwi/tractography/connectome/matrix.cpp @@ -18,21 +18,20 @@ #include +#include "enum.h" #include "file/matrix.h" #include "file/path.h" namespace MR::DWI::Tractography::Connectome { -std::vector statistics = {"sum", "mean", "min", "max"}; - const App::Option EdgeStatisticOption = App::Option("stat_edge", "statistic for combining the values from all streamlines in an edge " "into a single scale value for that edge " "(options are: " + - join(statistics, ",") + "; default=sum)") + - App::Argument("statistic").type_choice(statistics); + MR::Enum::join(",") + "; default=sum)") + + App::Argument("statistic").type_choice(); template bool Matrix::operator()(const Mapped_track_nodepair &in) { assert(assignments_lists.empty()); diff --git a/cpp/core/dwi/tractography/connectome/matrix.h b/cpp/core/dwi/tractography/connectome/matrix.h index a2a9864451..6ac4652e97 100644 --- a/cpp/core/dwi/tractography/connectome/matrix.h +++ b/cpp/core/dwi/tractography/connectome/matrix.h @@ -31,7 +31,6 @@ namespace MR::DWI::Tractography::Connectome { enum stat_edge { SUM, MEAN, MIN, MAX }; -extern std::vector statistics; extern const App::Option EdgeStatisticOption; // The number of nodes that must be exceeded in a connectome matrix in diff --git a/cpp/core/dwi/tractography/mapping/twi_stats.cpp b/cpp/core/dwi/tractography/mapping/twi_stats.cpp index 7ddd0d7560..ae6113aebb 100644 --- a/cpp/core/dwi/tractography/mapping/twi_stats.cpp +++ b/cpp/core/dwi/tractography/mapping/twi_stats.cpp @@ -30,13 +30,9 @@ const std::unordered_map contrast_names{ {contrast_t::CURVATURE, {"curvature", "curvature}"}}, {contrast_t::VECTOR_FILE, {"vector_file", "external-file-based"}}}; -const std::vector contrasts = { - "tdi", "length", "invlength", "scalar_map", "scalar_map_count", "fod_amp", "curvature", "vector_file"}; - // TODO Remove "V_" prefix const std::unordered_map voxel_statistic_names{ {vox_stat_t::SUM, "sum"}, {vox_stat_t::MIN, "mininum"}, {vox_stat_t::MEAN, "mean"}, {vox_stat_t::MAX, "maximum"}}; -const std::vector voxel_statistics = {"sum", "min", "mean", "max"}; const std::unordered_map track_statistic_names{ {tck_stat_t::SUM, {"sum", "sum"}}, @@ -52,17 +48,4 @@ const std::unordered_map track_statistic_names{ {tck_stat_t::ENDS_PROD, {"ends_prod", "product-of-endpoints"}}, {tck_stat_t::ENDS_CORR, {"ends_corr", "correlation-between-endpoints"}}}; -// Note: ENDS_CORR not provided as a command-line option -const std::vector track_statistics = {"sum", - "min", - "mean", - "max", - "median", - "mean_nonzero", - "gaussian", - "ends_min", - "ends_mean", - "ends_max", - "ends_prod"}; - } // namespace MR::DWI::Tractography::Mapping diff --git a/cpp/core/dwi/tractography/mapping/twi_stats.h b/cpp/core/dwi/tractography/mapping/twi_stats.h index 3f052eba36..bcb6c8eb37 100644 --- a/cpp/core/dwi/tractography/mapping/twi_stats.h +++ b/cpp/core/dwi/tractography/mapping/twi_stats.h @@ -20,6 +20,8 @@ #include #include +#include + namespace MR::DWI::Tractography::Mapping { enum class contrast_t { TDI, LENGTH, INVLENGTH, SCALAR_MAP, SCALAR_MAP_COUNT, FOD_AMP, CURVATURE, VECTOR_FILE }; @@ -28,13 +30,12 @@ struct Strings { std::string description; }; extern const std::unordered_map contrast_names; -extern const std::vector contrasts; enum class vox_stat_t { SUM, MIN, MEAN, MAX }; extern const std::unordered_map voxel_statistic_names; -extern const std::vector voxel_statistics; -// Note: ENDS_CORR not provided as a command-line option +// Note: ENDS_CORR is meaningful internally (TW-dFC) but is excluded from magic_enum +// reflection below, so it is not offered as a command-line option. enum class tck_stat_t { SUM, MIN, @@ -50,6 +51,20 @@ enum class tck_stat_t { ENDS_CORR }; extern const std::unordered_map track_statistic_names; -extern const std::vector track_statistics; } // namespace MR::DWI::Tractography::Mapping + +// Exclude ENDS_CORR from magic_enum reflection of tck_stat_t: it is used internally +// (TW-dFC) but is not a valid "-stat_tck" command-line choice. This makes +// type_choice() omit it from the presented choices and +// MR::Enum::from_name("ends_corr") reject it. The hand-written +// track_statistic_names map (twi_stats.cpp) retains ENDS_CORR for internal +// stringification, independently of magic_enum. +template <> +constexpr magic_enum::customize::customize_t +magic_enum::customize::enum_name( + MR::DWI::Tractography::Mapping::tck_stat_t value) noexcept { + return value == MR::DWI::Tractography::Mapping::tck_stat_t::ENDS_CORR // + ? magic_enum::customize::invalid_tag + : magic_enum::customize::default_tag; +} diff --git a/cpp/core/interp/interp.h b/cpp/core/interp/interp.h index 342ba04386..93d6bfd159 100644 --- a/cpp/core/interp/interp.h +++ b/cpp/core/interp/interp.h @@ -16,12 +16,8 @@ #pragma once -#include -#include - namespace MR::Interp { -const std::vector interp_choices{"nearest", "linear", "cubic", "sinc"}; enum class interp_type { NEAREST, LINEAR, CUBIC, SINC }; } // namespace MR::Interp diff --git a/cpp/core/math/average_space.cpp b/cpp/core/math/average_space.cpp index 29285437c1..9e5345884a 100644 --- a/cpp/core/math/average_space.cpp +++ b/cpp/core/math/average_space.cpp @@ -56,9 +56,6 @@ double matrix_average(std::vector const &mat_in, Eigen::MatrixX namespace MR { -const std::vector avgspace_voxspacing_choices{ - "min_projection", "mean_projection", "min_nearest", "mean_nearest"}; - FORCE_INLINE Eigen::Matrix get_cuboid_corners(const Eigen::Matrix &xyz_sizes) { Eigen::Matrix corners; // MatrixType faces = MatrixType(8,4); diff --git a/cpp/core/math/average_space.h b/cpp/core/math/average_space.h index 93d3e70247..21b66091f6 100644 --- a/cpp/core/math/average_space.h +++ b/cpp/core/math/average_space.h @@ -31,8 +31,6 @@ double matrix_average(std::vector const &mat_in, Eigen::MatrixX namespace MR { -extern const std::vector avgspace_voxspacing_choices; - enum class avgspace_voxspacing_t { MIN_PROJECTION, MEAN_PROJECTION, MIN_NEAREST, MEAN_NEAREST }; Eigen::Matrix get_cuboid_corners(const Eigen::Matrix &xyz_sizes); diff --git a/cpp/core/registration/linear.cpp b/cpp/core/registration/linear.cpp index 3272990dd2..f048d95520 100644 --- a/cpp/core/registration/linear.cpp +++ b/cpp/core/registration/linear.cpp @@ -16,18 +16,13 @@ #include +#include "enum.h" #include "registration/linear.h" namespace MR::Registration { using namespace App; -const std::vector initialisation_translation_choices = {"mass", "geometric", "none"}; -const std::vector initialisation_rotation_choices = {"search", "moments", "none"}; - -const std::vector linear_metric_choices = {"diff", "ncc"}; -const std::vector linear_robust_estimator_choices = {"l1", "l2", "lp", "none"}; -const std::vector linear_optimisation_algo_choices = {"bbgd", "gd"}; const std::vector optim_algo_names = {"BBGD", "GD"}; // define parameters of initialisation methods used for both, rigid and affine registration @@ -77,46 +72,37 @@ void parse_general_options(Registration::Linear ®istration) { opt = get_options("linstage.optimiser.default"); if (!opt.empty()) { - switch (static_cast(opt[0][0])) { - case 0: + switch (MR::Enum::from_name(opt[0][0])) { + case optimiser_choice_t::bbgd: registration.set_stage_optimiser_default(Registration::OptimiserAlgoType::bbgd); break; - case 1: + case optimiser_choice_t::gd: registration.set_stage_optimiser_default(Registration::OptimiserAlgoType::gd); break; - default: - assert(0 && "FIXME: linstage.optimiser.default not understood"); - break; } } opt = get_options("linstage.optimiser.first"); if (!opt.empty()) { - switch (static_cast(opt[0][0])) { - case 0: + switch (MR::Enum::from_name(opt[0][0])) { + case optimiser_choice_t::bbgd: registration.set_stage_optimiser_first(Registration::OptimiserAlgoType::bbgd); break; - case 1: + case optimiser_choice_t::gd: registration.set_stage_optimiser_first(Registration::OptimiserAlgoType::gd); break; - default: - assert(0 && "FIXME: linstage.optimiser.first not understood"); - break; } } opt = get_options("linstage.optimiser.last"); if (!opt.empty()) { - switch (static_cast(opt[0][0])) { - case 0: + switch (MR::Enum::from_name(opt[0][0])) { + case optimiser_choice_t::bbgd: registration.set_stage_optimiser_last(Registration::OptimiserAlgoType::bbgd); break; - case 1: + case optimiser_choice_t::gd: registration.set_stage_optimiser_last(Registration::OptimiserAlgoType::gd); break; - default: - assert(0 && "FIXME: linstage.optimiser.last not understood"); - break; } } @@ -136,36 +122,32 @@ void parse_general_options(Registration::Linear ®istration) { } } -void set_init_translation_model_from_option(Registration::Linear ®istration, const int &option) { +void set_init_translation_model_from_option(Registration::Linear ®istration, const init_translation_t option) { switch (option) { - case 0: + case init_translation_t::mass: registration.set_init_translation_type(Registration::Transform::Init::mass); break; - case 1: + case init_translation_t::geometric: registration.set_init_translation_type(Registration::Transform::Init::geometric); break; - case 2: + case init_translation_t::none: registration.set_init_translation_type(Registration::Transform::Init::none); break; - default: - break; } } -void set_init_rotation_model_from_option(Registration::Linear ®istration, const int &option) { +void set_init_rotation_model_from_option(Registration::Linear ®istration, const init_rotation_t option) { switch (option) { // TODO registration.set_init_type (Registration::Transform::Init::fod); - case 0: + case init_rotation_t::search: registration.set_init_rotation_type(Registration::Transform::Init::rot_search); break; - case 1: + case init_rotation_t::moments: registration.set_init_rotation_type(Registration::Transform::Init::moments); break; - case 2: + case init_rotation_t::none: registration.set_init_rotation_type(Registration::Transform::Init::none); break; - default: - break; } } @@ -231,7 +213,7 @@ const OptionGroup lin_stage_options = " bbgd (Barzilai-Borwein gradient descent);" " gd (simple gradient descent)." " (Default: bbgd)") - + Argument("algorithm").type_choice(linear_optimisation_algo_choices) + + Argument("algorithm").type_choice() + Option("linstage.optimiser.last", "Cost function optimisation algorithm to use at last iteration of all stages" " (if there are more than one)." @@ -239,7 +221,7 @@ const OptionGroup lin_stage_options = " bbgd (Barzilai-Borwein gradient descent);" " gd (simple gradient descent)." " (Default: bbgd)") - + Argument("algorithm").type_choice(linear_optimisation_algo_choices) + + Argument("algorithm").type_choice() + Option("linstage.optimiser.default", "Cost function optimisation algorithm to use at any stage iteration" " other than first or last iteration." @@ -247,7 +229,7 @@ const OptionGroup lin_stage_options = " bbgd (Barzilai-Borwein gradient descent);" " gd (simple gradient descent)." " (Default: bbgd)") - + Argument("algorithm").type_choice(linear_optimisation_algo_choices) + + Argument("algorithm").type_choice() + Option("linstage.diagnostics.dir", "generate diagnostics images after every registration stage") @@ -277,7 +259,7 @@ const OptionGroup rigid_options = " mass (aligns the centers of mass of both images, default);" " geometric (aligns geometric image centres);" " none.") - + Argument("type").type_choice(initialisation_translation_choices) + + Argument("type").type_choice() + Option("rigid_init_rotation", "Method to use to initialise the rotation." @@ -285,7 +267,7 @@ const OptionGroup rigid_options = " search (search for the best rotation using mean squared residuals);" // TODO CC " moments (rotation based on directions of intensity variance with respect to centre of mass);" " none (default).") // TODO This can be combined with rigid_init_translation. - + Argument("type").type_choice(initialisation_rotation_choices) + + Argument("type").type_choice() + Option("rigid_init_matrix", "initialise either the rigid, affine, or syn registration" @@ -313,7 +295,7 @@ const OptionGroup rigid_options = " diff (intensity differences);" // " ncc (normalised cross-correlation);" TODO " Default: diff") - + Argument("type").type_choice(linear_metric_choices) + + Argument("type").type_choice() + Option("rigid_metric.diff.estimator", "Robust estimator to use during rigid-body registration." @@ -323,7 +305,7 @@ const OptionGroup rigid_options = " lp (least powers: |x|^1.2);" " none." " Default: none.") - + Argument("type").type_choice(linear_robust_estimator_choices) + + Argument("type").type_choice() // + Option ("rigid_loop_density", "density of gradient descent 1 (batch) to 0.0 (max stochastic) (Default: 1.0)") // + Argument ("num").type_sequence_float () // TODO @@ -368,7 +350,7 @@ const OptionGroup affine_options = " geometric (aligns geometric image centres);" " none." " (Default: mass)") - + Argument("type").type_choice(initialisation_translation_choices) + + Argument("type").type_choice() + Option("affine_init_rotation", "initialise the rotation." @@ -377,7 +359,7 @@ const OptionGroup affine_options = " moments (rotation based on directions of intensity variance with respect to centre of mass);" " none" " (Default: none).") // TODO This can be combined with affine_init_translation. - + Argument("type").type_choice(initialisation_rotation_choices) + + Argument("type").type_choice() + Option("affine_init_matrix", "initialise either the affine or syn registration" @@ -404,7 +386,7 @@ const OptionGroup affine_options = " diff (intensity differences);" // "ncc (normalised cross-correlation) " TODO " Default: diff") - + Argument("type").type_choice(linear_metric_choices) + + Argument("type").type_choice() + Option("affine_metric.diff.estimator", "Robust estimator to use durring affine registration." @@ -414,7 +396,7 @@ const OptionGroup affine_options = " lp (least powers: |x|^1.2);" " none." " Default: none.") - + Argument("type").type_choice(linear_robust_estimator_choices) + + Argument("type").type_choice() // + Option ("affine_loop_density", "density of gradient descent 1 (batch) to 0.0 (max stochastic) (Default: 1.0)") // + Argument ("num").type_sequence_float () // TODO diff --git a/cpp/core/registration/linear.h b/cpp/core/registration/linear.h index 09e4f7f2d7..2f6221c193 100644 --- a/cpp/core/registration/linear.h +++ b/cpp/core/registration/linear.h @@ -56,6 +56,14 @@ enum LinearMetricType { Diff, NCC }; enum LinearRobustMetricEstimatorType { L1, L2, LP, None }; enum OptimiserAlgoType { bbgd, gd, none }; +// Command-line choice enumerations. +// The lowercase enumerator names define the set of valid choices for the +// corresponding command-line options; see linear.cpp. +enum class init_translation_t { mass, geometric, none }; +enum class init_rotation_t { search, moments, none }; +// Subset of OptimiserAlgoType selectable on the command-line (excludes 'none'). +enum class optimiser_choice_t { bbgd, gd }; + struct StageSetting { StageSetting() : stage_iterations(1), @@ -591,7 +599,7 @@ class Linear { Header midway_image_header; }; -void set_init_translation_model_from_option(Registration::Linear ®istration, const int &option); -void set_init_rotation_model_from_option(Registration::Linear ®istration, const int &option); +void set_init_translation_model_from_option(Registration::Linear ®istration, const init_translation_t option); +void set_init_rotation_model_from_option(Registration::Linear ®istration, const init_rotation_t option); void parse_general_options(Registration::Linear ®istration); } // namespace MR::Registration diff --git a/cpp/core/stats.cpp b/cpp/core/stats.cpp index 2b1169a98d..aa6bcae5bf 100644 --- a/cpp/core/stats.cpp +++ b/cpp/core/stats.cpp @@ -20,21 +20,19 @@ namespace MR::Stats { using namespace App; -const std::vector field_choices{"mean", "median", "std", "std_rv", "iqr", "min", "max", "count"}; - // clang-format off const OptionGroup Options = OptionGroup("Statistics options") + Option("output", "output only the field specified." " Multiple such options can be supplied if required." - " Choices are: " + join(field_choices, ", ") + "." + " Choices are: " + MR::Enum::join() + "." " Useful for use in scripts." " Both std options refer to the unbiased (sample) standard deviation." " For complex data, min, max and std are calculated separately for real and imaginary parts," " std_rv is based on the real valued variance" " (equals sqrt of sum of variances of imaginary and real parts).").allow_multiple() - + Argument("field").type_choice(field_choices) + + Argument("field").type_choice() + Option("mask", "only perform computation within the specified binary mask image.") + Argument("image").type_image_in() diff --git a/cpp/core/stats.h b/cpp/core/stats.h index cedc7e02de..3bdd10f11c 100644 --- a/cpp/core/stats.h +++ b/cpp/core/stats.h @@ -16,6 +16,7 @@ #pragma once +#include "enum.h" #include "math/median.h" #include "mrtrix.h" @@ -24,7 +25,8 @@ namespace MR::Stats { -extern const std::vector field_choices; +//! enumeration of the per-field statistics that can be requested via "-output" +enum class field_t { MEAN, MEDIAN, STD, STD_RV, IQR, MIN, MAX, COUNT }; extern const App::OptionGroup Options; using value_type = default_type; @@ -47,7 +49,7 @@ class Stats { void operator()(complex_type val); - template void print(ImageType &ima, const std::vector &fields) { + template void print(ImageType &ima, const std::vector &fields) { if (count > 1) { std = complex_type(sqrt(m2.real() / static_cast(count - 1)), @@ -57,7 +59,7 @@ class Stats { } if (!fields.empty()) { if (!count) { - if (fields.size() == 1 && fields.front() == "count") { + if (fields.size() == 1 && fields.front() == field_t::COUNT) { std::cout << "0\n"; return; } else { @@ -65,25 +67,33 @@ class Stats { } } for (size_t n = 0; n < fields.size(); ++n) { - if (fields[n] == "mean") + switch (fields[n]) { + case field_t::MEAN: std::cout << str(mean) << " "; - else if (fields[n] == "median") + break; + case field_t::MEDIAN: std::cout << (values.empty() ? "N/A" : str(Math::median(values))) << " "; - else if (fields[n] == "std") + break; + case field_t::STD: std::cout << (count > 1 ? str(std) : "N/A") << " "; - else if (fields[n] == "std_rv") + break; + case field_t::STD_RV: std::cout << (count > 1 ? str(std_rv) : "N/A") << " "; - else if (fields[n] == "iqr") + break; + case field_t::IQR: std::cout << (!values.empty() ? str(Math::quantile(values, 0.75) - Math::quantile(values, 0.25)) : "N/A") << " "; - else if (fields[n] == "min") + break; + case field_t::MIN: std::cout << str(min) << " "; - else if (fields[n] == "max") + break; + case field_t::MAX: std::cout << str(max) << " "; - else if (fields[n] == "count") + break; + case field_t::COUNT: std::cout << count << " "; - else - throw Exception("stats type not supported: " + fields[n]); + break; + } } std::cout << "\n"; diff --git a/cpp/gui/mrview/tool/tractography/tractography.cpp b/cpp/gui/mrview/tool/tractography/tractography.cpp index 2000aabbe2..fae794fd75 100644 --- a/cpp/gui/mrview/tool/tractography/tractography.cpp +++ b/cpp/gui/mrview/tool/tractography/tractography.cpp @@ -19,6 +19,7 @@ #include #include "dialog/file.h" +#include "enum.h" #include "lighting_dock.h" #include "mrtrix.h" #include "mrview/qthelpers.h" @@ -29,7 +30,6 @@ #include "opengl/lighting.h" namespace MR::GUI::MRView::Tool { -const std::vector tractogram_geometry_types = {"pseudotubes", "lines", "points"}; TrackGeometryType geometry_index2type(const int idx) { switch (idx) { @@ -45,18 +45,11 @@ TrackGeometryType geometry_index2type(const int idx) { } } +// The TrackGeometryType enumerators are declared in the same order as the +// geometry-type combobox entries, so the enumerator's underlying value +// doubles as the combobox index. size_t geometry_string2index(std::string type_str) { - type_str = lowercase(type_str); - - auto matches = [&type_str](std::string_view s) { return type_str == lowercase(s); }; - const auto &list = tractogram_geometry_types; - auto it = std::find_if(list.begin(), list.end(), matches); - if (it != list.end()) - return std::distance(list.begin(), it); - - throw Exception("Unrecognised value for tractogram geometry \"" + type_str + "\" (options are: " + join(list, ", ") + - "); ignoring"); - return 0; + return static_cast(MR::Enum::from_name(type_str)); } class Tractography::Model : public ListModelBase { @@ -288,7 +281,8 @@ Tractography::Tractography(Dock *parent) // CONF default: Pseudotubes // CONF The default geometry type used to render tractograms. // CONF Options are Pseudotubes, Lines or Points - const std::string default_geom_type = File::Config::get("MRViewDefaultTractGeomType", tractogram_geometry_types[0]); + const std::string default_geom_type = + File::Config::get("MRViewDefaultTractGeomType", MR::Enum::lowercase_name(TrackGeometryType::Pseudotubes)); try { const size_t default_geom_index = geometry_string2index(default_geom_type); Tractogram::default_tract_geom = geometry_index2type(default_geom_index); @@ -784,8 +778,8 @@ void Tractography::add_commandline_options(MR::App::OptionList &options) { + Option("tractography.geometry", "The geometry type to use when rendering tractograms" - " (options are: " + join(tractogram_geometry_types, ", ") + ")").allow_multiple() - + Argument("value").type_choice(tractogram_geometry_types) + " (options are: " + MR::Enum::join() + ")").allow_multiple() + + Argument("value").type_choice() + Option("tractography.opacity", "Opacity of tractography display, [0.0, 1.0];" diff --git a/cpp/gui/mrview/tool/tractography/tractography.h b/cpp/gui/mrview/tool/tractography/tractography.h index 69ba7ec304..a1f8dcd407 100644 --- a/cpp/gui/mrview/tool/tractography/tractography.h +++ b/cpp/gui/mrview/tool/tractography/tractography.h @@ -33,8 +33,6 @@ class LightingDock; namespace MR::GUI::MRView::Tool { -extern const std::vector tractogram_geometry_types; - class Tractography : public Base { Q_OBJECT diff --git a/testing/data/cpp_cli/full_usage.txt b/testing/data/cpp_cli/full_usage.txt index 46af116b22..1f868ec819 100644 --- a/testing/data/cpp_cli/full_usage.txt +++ b/testing/data/cpp_cli/full_usage.txt @@ -33,7 +33,7 @@ a comma-separated sequence of floating-point numbers ARGUMENT values 0 0 FSEQ OPTION choice 1 0 a choice from a set of options -ARGUMENT item 0 0 CHOICE One Two Three +ARGUMENT item 0 0 CHOICE one two three OPTION file_in 1 0 an input file ARGUMENT input 0 0 FILEIN @@ -54,7 +54,7 @@ an output tractogram ARGUMENT output 0 0 TRACKSOUT OPTION any 1 0 an argument that could accept any of the various forms -ARGUMENT spec 0 0 TEXT BOOL INT -9223372036854775808 9223372036854775807 FLOAT -inf inf FILEIN FILEOUT DIRIN DIROUT ISEQ FSEQ TRACKSIN TRACKSOUT CHOICE One Two Three +ARGUMENT spec 0 0 TEXT BOOL INT -9223372036854775808 9223372036854775807 FLOAT -inf inf FILEIN FILEOUT DIRIN DIROUT ISEQ FSEQ TRACKSIN TRACKSOUT CHOICE one two three OPTION nargs_two 1 0 A command-line option that accepts two arguments ARGUMENT first 0 0 TEXT diff --git a/testing/tools/testing_cpp_cli.cpp b/testing/tools/testing_cpp_cli.cpp index ceaddbdcff..bc8dbdef96 100644 --- a/testing/tools/testing_cpp_cli.cpp +++ b/testing/tools/testing_cpp_cli.cpp @@ -22,7 +22,7 @@ using namespace MR; using namespace App; -const std::vector choices = {"One", "Two", "Three"}; +enum class Choice { One, Two, Three }; // clang-format off void usage() { @@ -67,7 +67,7 @@ void usage() { + Argument("values").type_sequence_float() + Option("choice", "a choice from a set of options") - + Argument("item").type_choice(choices) + + Argument("item").type_choice() + Option("file_in", "an input file") + Argument("input").type_file_in() @@ -94,7 +94,7 @@ void usage() { .type_float() .type_sequence_int() .type_sequence_float() - .type_choice(choices) + .type_choice() .type_file_in() .type_file_out() .type_directory_in() From 292ecaad5e096d13a9e748e71fefea9c09a12834 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Wed, 3 Jun 2026 20:19:16 +1000 Subject: [PATCH 2/5] Manual cleanups reviewing removal of MR::Argument::type_choice(std::vector) - Somewhat unrelated but code came up during review: Change MR::File::Config::get(std::string) to return a std::optional. - Remove some redundant enums in registration code, and collapse unnecessary if branches. - Add code comments identifying enum usages that should be refactored. --- .../dwi/tractography/mapping/twi_stats.cpp | 1 - cpp/core/file/config.cpp | 55 ++++++++-------- cpp/core/file/config.h | 2 +- cpp/core/registration/linear.cpp | 64 ++++++------------- cpp/core/registration/linear.h | 22 +++---- cpp/core/stats.h | 19 +++--- cpp/core/thread.cpp | 2 +- cpp/gui/mrview/mode/volume.cpp | 6 +- .../mrview/tool/tractography/tractography.cpp | 22 ++++--- cpp/gui/mrview/window.cpp | 17 ++--- docs/reference/commands/mrregister.rst | 6 +- 11 files changed, 98 insertions(+), 118 deletions(-) diff --git a/cpp/core/dwi/tractography/mapping/twi_stats.cpp b/cpp/core/dwi/tractography/mapping/twi_stats.cpp index ae6113aebb..51b896a7b8 100644 --- a/cpp/core/dwi/tractography/mapping/twi_stats.cpp +++ b/cpp/core/dwi/tractography/mapping/twi_stats.cpp @@ -30,7 +30,6 @@ const std::unordered_map contrast_names{ {contrast_t::CURVATURE, {"curvature", "curvature}"}}, {contrast_t::VECTOR_FILE, {"vector_file", "external-file-based"}}}; -// TODO Remove "V_" prefix const std::unordered_map voxel_statistic_names{ {vox_stat_t::SUM, "sum"}, {vox_stat_t::MIN, "mininum"}, {vox_stat_t::MEAN, "mean"}, {vox_stat_t::MAX, "maximum"}}; diff --git a/cpp/core/file/config.cpp b/cpp/core/file/config.cpp index c4e2fc63a1..375235df45 100644 --- a/cpp/core/file/config.cpp +++ b/cpp/core/file/config.cpp @@ -14,6 +14,8 @@ * For more details, see http://www.mrtrix.org/. */ +#include + #include "app.h" #include "debug.h" #include "env.h" @@ -80,9 +82,9 @@ void Config::init() { Header::do_realign_transform = get_bool("RealignTransform", true); } -std::string Config::get(std::string_view key) { +std::optional Config::get(std::string_view key) { const KeyValues::const_iterator i = config.find(std::string(key)); - return (i != config.end() ? i->second : ""); + return (i != config.end() ? std::optional(i->second) : std::nullopt); } std::string Config::get(std::string_view key, std::string_view default_value) { @@ -91,54 +93,55 @@ std::string Config::get(std::string_view key, std::string_view default_value) { } bool Config::get_bool(std::string_view key, bool default_value) { - const std::string value = get(std::string(key)); - if (value.empty()) + const auto from_config = get(std::string(key)); + if (!from_config.has_value()) return default_value; try { - return to(value); + return to(from_config.value()); } catch (...) { - WARN("malformed boolean entry \"" + value + "\" for key \"" + key + "\" in configuration file - ignored"); + WARN("malformed boolean entry \"" + from_config.value() + "\" for key \"" + key + "\"" + // + " in configuration file - ignored"); return default_value; } } int Config::get_int(std::string_view key, int default_value) { - const std::string value = get(std::string(key)); - if (value.empty()) + const auto from_config = get(std::string(key)); + if (!from_config.has_value()) return default_value; try { - return to(value); + return to(from_config.value()); } catch (...) { - WARN("malformed integer entry \"" + value + "\" for key \"" + key + "\" in configuration file - ignored"); + WARN("malformed integer entry \"" + from_config.value() + "\" for key \"" + key + "\"" + // + " in configuration file - ignored"); return default_value; } } float Config::get_float(std::string_view key, float default_value) { - const std::string value = get(std::string(key)); - if (value.empty()) + const auto from_config = get(std::string(key)); + if (!from_config.has_value()) return default_value; try { - return to(value); + return to(from_config.value()); } catch (...) { - WARN("malformed floating-point entry \"" + value + "\" for key \"" + key + "\" in configuration file - ignored"); + WARN("malformed floating-point entry \"" + from_config.value() + "\" for key \"" + key + "\"" + // + " in configuration file - ignored"); return default_value; } } Eigen::Array3f Config::get_RGB(std::string_view key, const Eigen::Array3f &default_value) { - const std::string value = get(std::string(key)); - if (!value.empty()) { - try { - std::vector V(parse_floats(value)); - if (V.size() < 3) - throw Exception("malformed RGB entry \"" + value + "\" for key \"" + key + - "\" in configuration file - ignored"); - return {static_cast(V[0]), static_cast(V[1]), static_cast(V[2])}; - } catch (Exception) { - return default_value; - } - } else { + const auto from_config = get(std::string(key)); + if (!from_config.has_value()) + return default_value; + try { + std::vector V(parse_floats(from_config.value())); + if (V.size() < 3) + throw Exception("malformed RGB entry \"" + from_config.value() + "\" for key \"" + key + "\"" + // + "in configuration file - ignored"); + return {static_cast(V[0]), static_cast(V[1]), static_cast(V[2])}; + } catch (Exception) { return default_value; } } diff --git a/cpp/core/file/config.h b/cpp/core/file/config.h index b2460f58b4..a47cc720c2 100644 --- a/cpp/core/file/config.h +++ b/cpp/core/file/config.h @@ -28,7 +28,7 @@ class Config { static void init(); static void set(std::string_view key, std::string_view value) { config[std::string(key)] = std::string(value); } - static std::string get(std::string_view key); + static std::optional get(std::string_view key); static std::string get(std::string_view key, std::string_view default_value); static bool get_bool(std::string_view key, bool default_value); static int get_int(std::string_view key, int default_value); diff --git a/cpp/core/registration/linear.cpp b/cpp/core/registration/linear.cpp index f048d95520..78ed978978 100644 --- a/cpp/core/registration/linear.cpp +++ b/cpp/core/registration/linear.cpp @@ -23,8 +23,6 @@ namespace MR::Registration { using namespace App; -const std::vector optim_algo_names = {"BBGD", "GD"}; - // define parameters of initialisation methods used for both, rigid and affine registration void parse_general_options(Registration::Linear ®istration) { if (!get_options("init_translation.unmasked1").empty()) @@ -71,40 +69,14 @@ void parse_general_options(Registration::Linear ®istration) { } opt = get_options("linstage.optimiser.default"); - if (!opt.empty()) { - switch (MR::Enum::from_name(opt[0][0])) { - case optimiser_choice_t::bbgd: - registration.set_stage_optimiser_default(Registration::OptimiserAlgoType::bbgd); - break; - case optimiser_choice_t::gd: - registration.set_stage_optimiser_default(Registration::OptimiserAlgoType::gd); - break; - } - } - + if (!opt.empty()) + registration.set_stage_optimiser_default(MR::Enum::from_name(opt[0][0])); opt = get_options("linstage.optimiser.first"); - if (!opt.empty()) { - switch (MR::Enum::from_name(opt[0][0])) { - case optimiser_choice_t::bbgd: - registration.set_stage_optimiser_first(Registration::OptimiserAlgoType::bbgd); - break; - case optimiser_choice_t::gd: - registration.set_stage_optimiser_first(Registration::OptimiserAlgoType::gd); - break; - } - } - + if (!opt.empty()) + registration.set_stage_optimiser_first(MR::Enum::from_name(opt[0][0])); opt = get_options("linstage.optimiser.last"); - if (!opt.empty()) { - switch (MR::Enum::from_name(opt[0][0])) { - case optimiser_choice_t::bbgd: - registration.set_stage_optimiser_last(Registration::OptimiserAlgoType::bbgd); - break; - case optimiser_choice_t::gd: - registration.set_stage_optimiser_last(Registration::OptimiserAlgoType::gd); - break; - } - } + if (!opt.empty()) + registration.set_stage_optimiser_last(MR::Enum::from_name(opt[0][0])); opt = get_options("linstage.iterations"); if (!opt.empty()) { @@ -210,26 +182,26 @@ const OptionGroup lin_stage_options = + Option("linstage.optimiser.first", "Cost function optimisation algorithm to use at first iteration of all stages." " Valid choices:" - " bbgd (Barzilai-Borwein gradient descent);" - " gd (simple gradient descent)." - " (Default: bbgd)") - + Argument("algorithm").type_choice() + " BBGD (Barzilai-Borwein gradient descent);" + " GD (simple gradient descent)." + " (Default: BBGD)") + + Argument("algorithm").type_choice() + Option("linstage.optimiser.last", "Cost function optimisation algorithm to use at last iteration of all stages" " (if there are more than one)." " Valid choices:" - " bbgd (Barzilai-Borwein gradient descent);" - " gd (simple gradient descent)." - " (Default: bbgd)") - + Argument("algorithm").type_choice() + " BBGD (Barzilai-Borwein gradient descent);" + " GD (simple gradient descent)." + " (Default: BBGD)") + + Argument("algorithm").type_choice() + Option("linstage.optimiser.default", "Cost function optimisation algorithm to use at any stage iteration" " other than first or last iteration." " Valid choices:" - " bbgd (Barzilai-Borwein gradient descent);" - " gd (simple gradient descent)." - " (Default: bbgd)") - + Argument("algorithm").type_choice() + " BBGD (Barzilai-Borwein gradient descent);" + " GD (simple gradient descent)." + " (Default: BBGD)") + + Argument("algorithm").type_choice() + Option("linstage.diagnostics.dir", "generate diagnostics images after every registration stage") diff --git a/cpp/core/registration/linear.h b/cpp/core/registration/linear.h index 2f6221c193..94e37820fc 100644 --- a/cpp/core/registration/linear.h +++ b/cpp/core/registration/linear.h @@ -50,29 +50,29 @@ extern const App::OptionGroup lin_stage_options; extern const App::OptionGroup rigid_options; extern const App::OptionGroup affine_options; extern const App::OptionGroup fod_options; -extern const std::vector optim_algo_names; enum LinearMetricType { Diff, NCC }; enum LinearRobustMetricEstimatorType { L1, L2, LP, None }; -enum OptimiserAlgoType { bbgd, gd, none }; +enum OptimiserAlgoType { BBGD, GD }; // Command-line choice enumerations. // The lowercase enumerator names define the set of valid choices for the // corresponding command-line options; see linear.cpp. +// TODO Define these as subsets of MR::Registration::Transform::Init::InitType +// as is performed in cpp/core/dwi/tractography/mapping/twi_stats.h using: +// constexpr magic_enum::customize::customize_t magic_enum::customize::enum_name<>(); enum class init_translation_t { mass, geometric, none }; enum class init_rotation_t { search, moments, none }; -// Subset of OptimiserAlgoType selectable on the command-line (excludes 'none'). -enum class optimiser_choice_t { bbgd, gd }; struct StageSetting { StageSetting() : stage_iterations(1), gd_max_iter(500), scale_factor(1.0), - optimisers(1, OptimiserAlgoType::bbgd), - optimiser_default(OptimiserAlgoType::bbgd), - optimiser_first(OptimiserAlgoType::bbgd), - optimiser_last(OptimiserAlgoType::gd), + optimisers(1, OptimiserAlgoType::BBGD), + optimiser_default(OptimiserAlgoType::BBGD), + optimiser_first(OptimiserAlgoType::BBGD), + optimiser_last(OptimiserAlgoType::GD), loop_density(1.0), fod_lmax(-1) {} @@ -87,9 +87,7 @@ struct StageSetting { if (stage_iterations > 1) st += ", iterations: " + str(stage_iterations); st += ", optimiser: "; - for (auto &optim : optimisers) { - st += str(optim_algo_names[optim]) + " "; - } + st += MR::join(MR::Enum::lower_case_names(), " "); return st; } size_t stage_iterations, gd_max_iter; @@ -508,7 +506,7 @@ class Linear { INFO("registration stage running..."); for (auto stage_iter = 1U; stage_iter <= stage.stage_iterations; ++stage_iter) { - if (stage.gd_max_iter > 0 and stage.optimisers[stage_iter - 1] == OptimiserAlgoType::bbgd) { + if (stage.gd_max_iter > 0 and stage.optimisers[stage_iter - 1] == OptimiserAlgoType::BBGD) { Math::GradientDescentBB, typename TransformType::UpdateType> optim( evaluate, *transform.get_gradient_descent_updator()); optim.be_verbose(analyse_descent); diff --git a/cpp/core/stats.h b/cpp/core/stats.h index 3bdd10f11c..a84614d4ed 100644 --- a/cpp/core/stats.h +++ b/cpp/core/stats.h @@ -69,31 +69,32 @@ class Stats { for (size_t n = 0; n < fields.size(); ++n) { switch (fields[n]) { case field_t::MEAN: - std::cout << str(mean) << " "; + std::cout << str(mean); break; case field_t::MEDIAN: - std::cout << (values.empty() ? "N/A" : str(Math::median(values))) << " "; + std::cout << (values.empty() ? "N/A" : str(Math::median(values))); break; case field_t::STD: - std::cout << (count > 1 ? str(std) : "N/A") << " "; + std::cout << (count > 1 ? str(std) : "N/A"); break; case field_t::STD_RV: - std::cout << (count > 1 ? str(std_rv) : "N/A") << " "; + std::cout << (count > 1 ? str(std_rv) : "N/A"); break; case field_t::IQR: - std::cout << (!values.empty() ? str(Math::quantile(values, 0.75) - Math::quantile(values, 0.25)) : "N/A") - << " "; + std::cout << (!values.empty() ? str(Math::quantile(values, 0.75) - Math::quantile(values, 0.25)) : "N/A"); break; case field_t::MIN: - std::cout << str(min) << " "; + std::cout << str(min); break; case field_t::MAX: - std::cout << str(max) << " "; + std::cout << str(max); break; case field_t::COUNT: - std::cout << count << " "; + std::cout << count; break; } + if (n < fields.size() - 1) + std::cout << " "; } std::cout << "\n"; diff --git a/cpp/core/thread.cpp b/cpp/core/thread.cpp index 810c764b45..cc2f6cdbda 100644 --- a/cpp/core/thread.cpp +++ b/cpp/core/thread.cpp @@ -63,7 +63,7 @@ size_t number_of_threads() { // CONF option: NumberOfThreads // CONF default: number of threads provided by hardware // CONF Set the default number of CPU threads to use for multi-threading. - if (!File::Config::get("NumberOfThreads").empty()) { + if (File::Config::get("NumberOfThreads").has_value()) { const int i = File::Config::get_int("NumberOfThreads", -1); if (i >= 0) { _number_of_threads = i; diff --git a/cpp/gui/mrview/mode/volume.cpp b/cpp/gui/mrview/mode/volume.cpp index 1d805fd8ab..319b5ef2f1 100644 --- a/cpp/gui/mrview/mode/volume.cpp +++ b/cpp/gui/mrview/mode/volume.cpp @@ -50,10 +50,10 @@ std::string Volume::Shader::vertex_shader_source(const Displayable &) { std::string Volume::Shader::fragment_shader_source(const Displayable &object) { std::vector> clip = mode.get_active_clip_planes(); const bool AND = mode.get_clipintersectionmodestate(); - std::string clip_color_spec = File::Config::get("MRViewClipPlaneColour"); + const auto clip_color_spec = File::Config::get("MRViewClipPlaneColour"); std::vector clip_color = {1.0, 0.0, 0.0, 0.1}; - if (!clip_color_spec.empty()) { - auto colour = parse_floats(clip_color_spec); + if (clip_color_spec.has_value()) { + auto colour = parse_floats(clip_color_spec.value()); if (colour.size() != 4) WARN("malformed config file entry for \"MRViewClipPlaneColour\" - expected 4 comma-separated values"); clip_color = {static_cast(colour[0]), diff --git a/cpp/gui/mrview/tool/tractography/tractography.cpp b/cpp/gui/mrview/tool/tractography/tractography.cpp index fae794fd75..8338ca8564 100644 --- a/cpp/gui/mrview/tool/tractography/tractography.cpp +++ b/cpp/gui/mrview/tool/tractography/tractography.cpp @@ -31,6 +31,7 @@ namespace MR::GUI::MRView::Tool { +// TODO Delete me: replace with magic_enum functionality TrackGeometryType geometry_index2type(const int idx) { switch (idx) { case 0: @@ -48,6 +49,7 @@ TrackGeometryType geometry_index2type(const int idx) { // The TrackGeometryType enumerators are declared in the same order as the // geometry-type combobox entries, so the enumerator's underlying value // doubles as the combobox index. +// TODO Delete me; use magic_enum, throw Exception if no match size_t geometry_string2index(std::string type_str) { return static_cast(MR::Enum::from_name(type_str)); } @@ -183,6 +185,8 @@ Tractography::Tractography(Dock *parent) geom_type_combobox = new ComboBoxWithErrorMsg(this, "(variable)"); geom_type_combobox->setToolTip(tr("Set the tractogram geometry type")); + // TODO Guarantee that these are inserted in the same order as the enum + // by looping over the enum itself geom_type_combobox->addItem("Pseudotubes"); geom_type_combobox->addItem("Lines"); geom_type_combobox->addItem("Points"); @@ -277,19 +281,21 @@ Tractography::Tractography(Dock *parent) connect(action, SIGNAL(triggered()), this, SLOT(colour_by_scalar_file_slot())); track_option_menu->addAction(action); + Tractogram::default_tract_geom = TrackGeometryType::Pseudotubes; // CONF option: MRViewDefaultTractGeomType // CONF default: Pseudotubes // CONF The default geometry type used to render tractograms. // CONF Options are Pseudotubes, Lines or Points - const std::string default_geom_type = - File::Config::get("MRViewDefaultTractGeomType", MR::Enum::lowercase_name(TrackGeometryType::Pseudotubes)); - try { - const size_t default_geom_index = geometry_string2index(default_geom_type); - Tractogram::default_tract_geom = geometry_index2type(default_geom_index); - geom_type_combobox->setCurrentIndex(default_geom_index); - } catch (Exception &e) { - e.display(); + auto default_geom_type_config = File::Config::get("MRViewDefaultTractGeomType"); + if (default_geom_type_config.has_value()) { + try { + const size_t default_geom_index = geometry_string2index(default_geom_type_config.value()); + Tractogram::default_tract_geom = geometry_index2type(default_geom_index); + } catch (Exception &e) { + e.display(); + } } + geom_type_combobox->setCurrentIndex(*magic_enum::enum_index(Tractogram::default_tract_geom)); // In the instance where pseudotubes are _not_ the default, enable lighting by default if (Tractogram::default_tract_geom != TrackGeometryType::Pseudotubes) { diff --git a/cpp/gui/mrview/window.cpp b/cpp/gui/mrview/window.cpp index 4a616848f1..2dc288df02 100644 --- a/cpp/gui/mrview/window.cpp +++ b/cpp/gui/mrview/window.cpp @@ -67,10 +67,10 @@ template <> inline QPoint position(QWheelEvent *event) { } Qt::KeyboardModifiers get_modifier(std::string_view key, Qt::KeyboardModifiers default_key) { - std::string value = lowercase(MR::File::Config::get(key)); - if (value.empty()) + const auto from_config = MR::File::Config::get(key); + if (!from_config.has_value()) return default_key; - + const std::string value = lowercase(from_config.value()); if (value == "shift") return Qt::ShiftModifier; if (value == "alt") @@ -141,10 +141,10 @@ QSize Window::GLArea::sizeHint() const { // CONF option: MRViewInitWindowSize // CONF Initial window size of MRView in pixels. // CONF default: 512,512 - std::string init_size_string = lowercase(MR::File::Config::get("MRViewInitWindowSize")); + const auto from_config = MR::File::Config::get("MRViewInitWindowSize"); std::vector init_window_size; - if (init_size_string.length()) - init_window_size = parse_ints(init_size_string); + if (from_config.has_value()) + init_window_size = parse_ints(from_config.value()); if (init_window_size.size() == 2) return QSize(init_window_size[0], init_window_size[1]); else @@ -267,8 +267,9 @@ Window::Window() // CONF top, bottom, left, right. Qt::ToolBarArea toolbar_position = Qt::TopToolBarArea; { - std::string toolbar_pos_spec = lowercase(MR::File::Config::get("InitialToolBarPosition")); - if (!toolbar_pos_spec.empty()) { + const auto from_config = MR::File::Config::get("InitialToolBarPosition"); + if (from_config.has_value()) { + const std::string toolbar_pos_spec = lowercase(from_config.value()); if (toolbar_pos_spec == "bottom") toolbar_position = Qt::BottomToolBarArea; else if (toolbar_pos_spec == "left") diff --git a/docs/reference/commands/mrregister.rst b/docs/reference/commands/mrregister.rst index 95c01a37e7..ed4fc4f189 100644 --- a/docs/reference/commands/mrregister.rst +++ b/docs/reference/commands/mrregister.rst @@ -122,11 +122,11 @@ Advanced linear registration stage options - **-linstage.iterations value(s)** number of iterations for each registration stage. Not to be confused with -rigid_niter or -affine_niter. This can be used to generate intermediate diagnostics images (-linstage.diagnostics.dir) or to change the cost function optimiser during registration (without the need to repeatedly resize the images). (Default: 1 == no repetition) -- **-linstage.optimiser.first algorithm** Cost function optimisation algorithm to use at first iteration of all stages. Valid choices: bbgd (Barzilai-Borwein gradient descent); gd (simple gradient descent). (Default: bbgd) +- **-linstage.optimiser.first algorithm** Cost function optimisation algorithm to use at first iteration of all stages. Valid choices: BBGD (Barzilai-Borwein gradient descent); GD (simple gradient descent). (Default: BBGD) -- **-linstage.optimiser.last algorithm** Cost function optimisation algorithm to use at last iteration of all stages (if there are more than one). Valid choices: bbgd (Barzilai-Borwein gradient descent); gd (simple gradient descent). (Default: bbgd) +- **-linstage.optimiser.last algorithm** Cost function optimisation algorithm to use at last iteration of all stages (if there are more than one). Valid choices: BBGD (Barzilai-Borwein gradient descent); GD (simple gradient descent). (Default: BBGD) -- **-linstage.optimiser.default algorithm** Cost function optimisation algorithm to use at any stage iteration other than first or last iteration. Valid choices: bbgd (Barzilai-Borwein gradient descent); gd (simple gradient descent). (Default: bbgd) +- **-linstage.optimiser.default algorithm** Cost function optimisation algorithm to use at any stage iteration other than first or last iteration. Valid choices: BBGD (Barzilai-Borwein gradient descent); GD (simple gradient descent). (Default: BBGD) - **-linstage.diagnostics.dir dir** generate diagnostics images after every registration stage From bdc1e4b641dece38223e4496e92379ecf7963854 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Wed, 3 Jun 2026 20:57:00 +1000 Subject: [PATCH 3/5] Replace tractography geometry-type enum helpers with magic_enum Remove the bespoke geometry_index2type() switch and the geometry_string2index() wrapper in the mrview tractography tool, both of which duplicated functionality already provided by the magic_enum library. Index-to-enum conversions now call magic_enum::enum_value, and string-to-enum conversions call MR::Enum::from_name directly at each call site, with the single-line helpers deleted entirely. The geometry combobox is now populated by looping over magic_enum::enum_names so its entry order is guaranteed to track the enumerator order. Session prompts: 1. > Scan HEAD comment for TODO comments in directory > cpp/gui/mrview/tool/tractography/ relating to sub-optimal use of > enumerations. For each, determine the appropriate corresponding > functionality in the magic_enum library and utilise. For any MRtrix > functions that collapse down to a single line of code, delete the > function and instead perform the computation inline in any invoking > code. Generated-by: Claude Opus 4.8 (1M context) --- .../mrview/tool/tractography/tractography.cpp | 39 ++++--------------- 1 file changed, 7 insertions(+), 32 deletions(-) diff --git a/cpp/gui/mrview/tool/tractography/tractography.cpp b/cpp/gui/mrview/tool/tractography/tractography.cpp index 8338ca8564..546bbb7711 100644 --- a/cpp/gui/mrview/tool/tractography/tractography.cpp +++ b/cpp/gui/mrview/tool/tractography/tractography.cpp @@ -31,29 +31,6 @@ namespace MR::GUI::MRView::Tool { -// TODO Delete me: replace with magic_enum functionality -TrackGeometryType geometry_index2type(const int idx) { - switch (idx) { - case 0: - return TrackGeometryType::Pseudotubes; - case 1: - return TrackGeometryType::Lines; - case 2: - return TrackGeometryType::Points; - default: - assert(0); - return TrackGeometryType::Pseudotubes; - } -} - -// The TrackGeometryType enumerators are declared in the same order as the -// geometry-type combobox entries, so the enumerator's underlying value -// doubles as the combobox index. -// TODO Delete me; use magic_enum, throw Exception if no match -size_t geometry_string2index(std::string type_str) { - return static_cast(MR::Enum::from_name(type_str)); -} - class Tractography::Model : public ListModelBase { public: @@ -185,11 +162,10 @@ Tractography::Tractography(Dock *parent) geom_type_combobox = new ComboBoxWithErrorMsg(this, "(variable)"); geom_type_combobox->setToolTip(tr("Set the tractogram geometry type")); - // TODO Guarantee that these are inserted in the same order as the enum - // by looping over the enum itself - geom_type_combobox->addItem("Pseudotubes"); - geom_type_combobox->addItem("Lines"); - geom_type_combobox->addItem("Points"); + // Insert combobox entries in the same order as the enumerators are declared, + // so that each entry's index matches its TrackGeometryType underlying value. + for (const auto &geom_type_name : magic_enum::enum_names()) + geom_type_combobox->addItem(qstr(geom_type_name)); connect(geom_type_combobox, SIGNAL(activated(int)), this, SLOT(geom_type_selection_slot(int))); hlayout->addWidget(geom_type_combobox); @@ -289,8 +265,7 @@ Tractography::Tractography(Dock *parent) auto default_geom_type_config = File::Config::get("MRViewDefaultTractGeomType"); if (default_geom_type_config.has_value()) { try { - const size_t default_geom_index = geometry_string2index(default_geom_type_config.value()); - Tractogram::default_tract_geom = geometry_index2type(default_geom_index); + Tractogram::default_tract_geom = MR::Enum::from_name(default_geom_type_config.value()); } catch (Exception &e) { e.display(); } @@ -646,7 +621,7 @@ void Tractography::geom_type_selection_slot(int selected_index) { if (selected_index == 3) return; - TrackGeometryType geom_type = geometry_index2type(selected_index); + TrackGeometryType geom_type = magic_enum::enum_value(static_cast(selected_index)); QModelIndexList indices = tractogram_list_view->selectionModel()->selectedIndexes(); for (int i = 0; i < indices.size(); ++i) @@ -973,7 +948,7 @@ bool Tractography::process_commandline_option(const MR::App::ParsedOption &opt) if (opt.opt->is("tractography.geometry")) { try { - const TrackGeometryType geom_type = geometry_index2type(geometry_string2index(opt[0])); + const TrackGeometryType geom_type = MR::Enum::from_name(std::string(opt[0])); QModelIndexList indices = tractogram_list_view->selectionModel()->selectedIndexes(); if (!indices.empty()) { for (int i = 0; i < indices.size(); ++i) From cc5f19714e5ee1a00f4597855352d860fb9a9f27 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Wed, 3 Jun 2026 23:23:22 +1000 Subject: [PATCH 4/5] Manual fixes to enum adoption #3379 --- cpp/cmd/mrmetric.cpp | 2 +- cpp/core/dwi/tractography/connectome/matrix.cpp | 8 ++++---- cpp/core/registration/linear.h | 6 ++---- cpp/gui/mrview/tool/tractography/tractography.cpp | 1 + docs/reference/commands/tck2connectome.rst | 2 +- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/cpp/cmd/mrmetric.cpp b/cpp/cmd/mrmetric.cpp index 271265a9ea..192a7846c5 100644 --- a/cpp/cmd/mrmetric.cpp +++ b/cpp/cmd/mrmetric.cpp @@ -199,7 +199,7 @@ void usage() { + Argument ("iteration method").type_choice() + Option ("interp", std::string("set the interpolation method to use when reslicing") + - " (choices: nearest, linear, cubic, sinc." + " (choices: " + MR::Enum::join() + "." " Default: " + MR::Enum::lowercase_name(default_interp) + ").") + Argument ("method").type_choice() diff --git a/cpp/core/dwi/tractography/connectome/matrix.cpp b/cpp/core/dwi/tractography/connectome/matrix.cpp index 07220d62f0..2b5b4f957a 100644 --- a/cpp/core/dwi/tractography/connectome/matrix.cpp +++ b/cpp/core/dwi/tractography/connectome/matrix.cpp @@ -24,14 +24,14 @@ namespace MR::DWI::Tractography::Connectome { +// clang-format off const App::Option EdgeStatisticOption - = App::Option("stat_edge", "statistic for combining the values from all streamlines in an edge " "into a single scale value for that edge " - "(options are: " + - MR::Enum::join(",") + "; default=sum)") + - App::Argument("statistic").type_choice(); + "(options are: " + MR::Enum::join() + "; default=sum)") + + App::Argument("statistic").type_choice(); +// clang-format on template bool Matrix::operator()(const Mapped_track_nodepair &in) { assert(assignments_lists.empty()); diff --git a/cpp/core/registration/linear.h b/cpp/core/registration/linear.h index 94e37820fc..5219107aba 100644 --- a/cpp/core/registration/linear.h +++ b/cpp/core/registration/linear.h @@ -58,9 +58,6 @@ enum OptimiserAlgoType { BBGD, GD }; // Command-line choice enumerations. // The lowercase enumerator names define the set of valid choices for the // corresponding command-line options; see linear.cpp. -// TODO Define these as subsets of MR::Registration::Transform::Init::InitType -// as is performed in cpp/core/dwi/tractography/mapping/twi_stats.h using: -// constexpr magic_enum::customize::customize_t magic_enum::customize::enum_name<>(); enum class init_translation_t { mass, geometric, none }; enum class init_rotation_t { search, moments, none }; @@ -87,7 +84,8 @@ struct StageSetting { if (stage_iterations > 1) st += ", iterations: " + str(stage_iterations); st += ", optimiser: "; - st += MR::join(MR::Enum::lower_case_names(), " "); + for (auto &optim : optimisers) + st += magic_enum::enum_name(optim) + " "; return st; } size_t stage_iterations, gd_max_iter; diff --git a/cpp/gui/mrview/tool/tractography/tractography.cpp b/cpp/gui/mrview/tool/tractography/tractography.cpp index 546bbb7711..03ec33c7e0 100644 --- a/cpp/gui/mrview/tool/tractography/tractography.cpp +++ b/cpp/gui/mrview/tool/tractography/tractography.cpp @@ -270,6 +270,7 @@ Tractography::Tractography(Dock *parent) e.display(); } } + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) geom_type_combobox->setCurrentIndex(*magic_enum::enum_index(Tractogram::default_tract_geom)); // In the instance where pseudotubes are _not_ the default, enable lighting by default diff --git a/docs/reference/commands/tck2connectome.rst b/docs/reference/commands/tck2connectome.rst index c27b06f2d0..172bf1448f 100644 --- a/docs/reference/commands/tck2connectome.rst +++ b/docs/reference/commands/tck2connectome.rst @@ -88,7 +88,7 @@ Options for outputting connectome matrices Other options for tck2connectome ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -- **-stat_edge statistic** statistic for combining the values from all streamlines in an edge into a single scale value for that edge (options are: sum,mean,min,max; default=sum) +- **-stat_edge statistic** statistic for combining the values from all streamlines in an edge into a single scale value for that edge (options are: sum, mean, min, max; default=sum) - **-tck_weights_in path** specify a text scalar file containing the streamline weights From 37ab55fa7b53ec1ef45057d8bb8ed71aa15d5dc5 Mon Sep 17 00:00:00 2001 From: Robert Smith Date: Thu, 4 Jun 2026 11:49:14 +1000 Subject: [PATCH 5/5] clang-tidy suggestions #3379 --- cpp/cmd/mraverageheader.cpp | 1 + cpp/cmd/mrgrid.cpp | 1 + cpp/cmd/mrmetric.cpp | 1 + cpp/cmd/mrregister.cpp | 1 + cpp/cmd/mrstats.cpp | 2 ++ cpp/cmd/mrtransform.cpp | 1 + cpp/cmd/tck2connectome.cpp | 2 +- cpp/cmd/tckdfc.cpp | 1 + cpp/cmd/tckmap.cpp | 3 +++ cpp/core/file/config.cpp | 5 ++++- cpp/core/file/config.h | 1 + cpp/core/registration/linear.h | 1 + cpp/core/stats.cpp | 1 + cpp/core/stats.h | 1 - cpp/gui/mrview/mode/volume.cpp | 2 ++ cpp/gui/mrview/tool/tractography/tractography.cpp | 6 +++++- cpp/gui/mrview/window.cpp | 2 ++ 17 files changed, 28 insertions(+), 4 deletions(-) diff --git a/cpp/cmd/mraverageheader.cpp b/cpp/cmd/mraverageheader.cpp index 499f567bbf..d7ea2be655 100644 --- a/cpp/cmd/mraverageheader.cpp +++ b/cpp/cmd/mraverageheader.cpp @@ -15,6 +15,7 @@ */ #include "algo/loop.h" +#include "app.h" #include "command.h" #include "debug.h" #include "enum.h" diff --git a/cpp/cmd/mrgrid.cpp b/cpp/cmd/mrgrid.cpp index 6dd09ac669..b1a878aba6 100644 --- a/cpp/cmd/mrgrid.cpp +++ b/cpp/cmd/mrgrid.cpp @@ -16,6 +16,7 @@ #include "adapter/regrid.h" #include "algo/copy.h" +#include "app.h" #include "command.h" #include "enum.h" #include "filter/resize.h" diff --git a/cpp/cmd/mrmetric.cpp b/cpp/cmd/mrmetric.cpp index 192a7846c5..f005b249e3 100644 --- a/cpp/cmd/mrmetric.cpp +++ b/cpp/cmd/mrmetric.cpp @@ -16,6 +16,7 @@ #include "algo/loop.h" #include "algo/threaded_loop.h" +#include "app.h" #include "command.h" #include "enum.h" #include "image.h" diff --git a/cpp/cmd/mrregister.cpp b/cpp/cmd/mrregister.cpp index 8943c74e8b..a1b10a7124 100644 --- a/cpp/cmd/mrregister.cpp +++ b/cpp/cmd/mrregister.cpp @@ -17,6 +17,7 @@ #include #include +#include "app.h" #include "command.h" #include "dwi/directions/predefined.h" #include "dwi/directions/validate.h" diff --git a/cpp/cmd/mrstats.cpp b/cpp/cmd/mrstats.cpp index bbb2574215..a447581c21 100644 --- a/cpp/cmd/mrstats.cpp +++ b/cpp/cmd/mrstats.cpp @@ -15,6 +15,7 @@ */ #include +#include #include "command.h" #include "datatype.h" @@ -111,6 +112,7 @@ void run() { std::vector fields; opt = get_options("output"); + fields.reserve(opt.size()); for (size_t n = 0; n < opt.size(); ++n) fields.push_back(MR::Enum::from_name(opt[n][0])); diff --git a/cpp/cmd/mrtransform.cpp b/cpp/cmd/mrtransform.cpp index 9c24269aa8..114b449669 100644 --- a/cpp/cmd/mrtransform.cpp +++ b/cpp/cmd/mrtransform.cpp @@ -21,6 +21,7 @@ #include "algo/copy.h" #include "algo/loop.h" #include "algo/threaded_copy.h" +#include "app.h" #include "command.h" #include "debug.h" #include "dwi/directions/predefined.h" diff --git a/cpp/cmd/tck2connectome.cpp b/cpp/cmd/tck2connectome.cpp index 4d4cf3d19f..e672d1616b 100644 --- a/cpp/cmd/tck2connectome.cpp +++ b/cpp/cmd/tck2connectome.cpp @@ -16,8 +16,8 @@ #include +#include "app.h" #include "command.h" -#include "enum.h" #include "image.h" #include "thread_queue.h" #include "types.h" diff --git a/cpp/cmd/tckdfc.cpp b/cpp/cmd/tckdfc.cpp index 3fd181124f..539c1d7f5b 100644 --- a/cpp/cmd/tckdfc.cpp +++ b/cpp/cmd/tckdfc.cpp @@ -16,6 +16,7 @@ #include +#include "app.h" #include "command.h" #include "enum.h" #include "exception.h" diff --git a/cpp/cmd/tckmap.cpp b/cpp/cmd/tckmap.cpp index a0328ee08a..8cf8cc88b7 100644 --- a/cpp/cmd/tckmap.cpp +++ b/cpp/cmd/tckmap.cpp @@ -16,7 +16,9 @@ #include +#include "app.h" #include "command.h" +#include "datatype.h" #include "enum.h" #include "image.h" #include "memory.h" @@ -32,6 +34,7 @@ #include "dwi/tractography/mapping/loader.h" #include "dwi/tractography/mapping/mapper.h" #include "dwi/tractography/mapping/mapping.h" +#include "dwi/tractography/mapping/twi_stats.h" #include "dwi/tractography/mapping/voxel.h" #include "dwi/tractography/mapping/writer.h" diff --git a/cpp/core/file/config.cpp b/cpp/core/file/config.cpp index 375235df45..ddfd0cd809 100644 --- a/cpp/core/file/config.cpp +++ b/cpp/core/file/config.cpp @@ -15,12 +15,15 @@ */ #include +#include #include "app.h" #include "debug.h" #include "env.h" #include "exception.h" #include "header.h" +#include "mrtrix.h" +#include "types.h" #include "file/config.h" #include "file/path.h" @@ -141,7 +144,7 @@ Eigen::Array3f Config::get_RGB(std::string_view key, const Eigen::Array3f &defau throw Exception("malformed RGB entry \"" + from_config.value() + "\" for key \"" + key + "\"" + // "in configuration file - ignored"); return {static_cast(V[0]), static_cast(V[1]), static_cast(V[2])}; - } catch (Exception) { + } catch (Exception &) { return default_value; } } diff --git a/cpp/core/file/config.h b/cpp/core/file/config.h index a47cc720c2..5368ef7617 100644 --- a/cpp/core/file/config.h +++ b/cpp/core/file/config.h @@ -18,6 +18,7 @@ #include #include +#include #include "file/key_value.h" #include "types.h" diff --git a/cpp/core/registration/linear.h b/cpp/core/registration/linear.h index 5219107aba..b476ac2261 100644 --- a/cpp/core/registration/linear.h +++ b/cpp/core/registration/linear.h @@ -16,6 +16,7 @@ #pragma once +#include "magic_enum/magic_enum.hpp" #include #include diff --git a/cpp/core/stats.cpp b/cpp/core/stats.cpp index aa6bcae5bf..b2697b8bab 100644 --- a/cpp/core/stats.cpp +++ b/cpp/core/stats.cpp @@ -15,6 +15,7 @@ */ #include "stats.h" +#include "enum.h" namespace MR::Stats { diff --git a/cpp/core/stats.h b/cpp/core/stats.h index a84614d4ed..186db37cf2 100644 --- a/cpp/core/stats.h +++ b/cpp/core/stats.h @@ -16,7 +16,6 @@ #pragma once -#include "enum.h" #include "math/median.h" #include "mrtrix.h" diff --git a/cpp/gui/mrview/mode/volume.cpp b/cpp/gui/mrview/mode/volume.cpp index 319b5ef2f1..9add13cd1c 100644 --- a/cpp/gui/mrview/mode/volume.cpp +++ b/cpp/gui/mrview/mode/volume.cpp @@ -15,7 +15,9 @@ */ #include "mrview/mode/volume.h" + #include "file/config.h" +#include "mrtrix.h" #include "mrview/adjust_button.h" #include "mrview/tool/base.h" #include "mrview/tool/view.h" diff --git a/cpp/gui/mrview/tool/tractography/tractography.cpp b/cpp/gui/mrview/tool/tractography/tractography.cpp index 03ec33c7e0..61b0121633 100644 --- a/cpp/gui/mrview/tool/tractography/tractography.cpp +++ b/cpp/gui/mrview/tool/tractography/tractography.cpp @@ -16,16 +16,20 @@ #include "mrview/tool/tractography/tractography.h" +#include "magic_enum/magic_enum.hpp" #include #include "dialog/file.h" #include "enum.h" +#include "file/config.h" +#include "gui.h" #include "lighting_dock.h" #include "mrtrix.h" #include "mrview/qthelpers.h" #include "mrview/tool/list_model_base.h" #include "mrview/tool/tractography/track_scalar_file.h" #include "mrview/tool/tractography/tractogram.h" +#include "mrview/tool/tractography/tractogram_enums.h" #include "mrview/window.h" #include "opengl/lighting.h" @@ -622,7 +626,7 @@ void Tractography::geom_type_selection_slot(int selected_index) { if (selected_index == 3) return; - TrackGeometryType geom_type = magic_enum::enum_value(static_cast(selected_index)); + const TrackGeometryType geom_type = magic_enum::enum_value(static_cast(selected_index)); QModelIndexList indices = tractogram_list_view->selectionModel()->selectedIndexes(); for (int i = 0; i < indices.size(); ++i) diff --git a/cpp/gui/mrview/window.cpp b/cpp/gui/mrview/window.cpp index 2dc288df02..76b5d167fe 100644 --- a/cpp/gui/mrview/window.cpp +++ b/cpp/gui/mrview/window.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include "algo/copy.h" @@ -28,6 +29,7 @@ #include "dialog/progress.h" #include "file/config.h" #include "header.h" +#include "mrtrix.h" #include "mrview/mode/base.h" #include "mrview/mode/list.h" #include "mrview/qthelpers.h"