From 14bfc19b6a217135d1df6b6fd887b0fa155da60d Mon Sep 17 00:00:00 2001 From: vhinger Date: Thu, 16 Jul 2026 19:21:09 +0200 Subject: [PATCH] Rework RemapAlgorithm.hpp according to comments - Add assert to flip - Only one SensorPlacement for strixel_to_pixel_maps - Pass order_map as NDView to ApplyRemap Additional: - Check output.shape() matches order_map.shape (correct buffer allocation) - Chash nrows and ncols - pass NDViews by value (copying is cheap and it makes the intent more clear that it only takes a snapshot of the arrays --- include/aare/RemapAlgorithm.hpp | 38 ++++++++++++++++++++++----------- src/RemapAlgorithm.cpp | 27 +++++++---------------- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/include/aare/RemapAlgorithm.hpp b/include/aare/RemapAlgorithm.hpp index e1f684c..b61265e 100644 --- a/include/aare/RemapAlgorithm.hpp +++ b/include/aare/RemapAlgorithm.hpp @@ -12,7 +12,8 @@ constexpr defs::Rotation flip(defs::Rotation r) noexcept { return defs::Rotation::Identity; } - return defs::Rotation::Identity; // or assert(false); + assert(false && "Invalid Rotation passed to flip"); + return defs::Rotation::Identity; // Unreachable; satisfies compiler } // Is it better to pass defs::SensorGroupConfig const& and return a copy? @@ -23,9 +24,10 @@ defs::StrixelGroupToPixelMap strixel_to_pixel_map( defs::SensorGroupConfig const &, defs::SensorPlacement const &, InclusiveROI const &user_roi, defs::BondShift bond_shift = {0, 0}); -std::vector strixel_to_pixel_maps( - defs::SensorConfig const &, std::vector const &, - InclusiveROI const &user_roi, defs::BondShift bond_shift = {0, 0}); +std::vector +strixel_to_pixel_maps(defs::SensorConfig const &, defs::SensorPlacement const &, + InclusiveROI const &user_roi, + defs::BondShift bond_shift = {0, 0}); defs::StrixelGroupToPixelMap combine_maps(std::vector const &, @@ -40,20 +42,32 @@ combine_maps(std::vector const &, * \param output Remapped array */ template -void ApplyRemap(NDView const &input, NDArray const &order_map, +void ApplyRemap(NDView input, NDView order_map, NDArray &output) { - for (size_t row = 0; row < order_map.shape(0); ++row) { - for (size_t col = 0; col < order_map.shape(1); ++col) { + + if (output.shape() != order_map.shape()) { + throw std::invalid_argument( + "ApplyRemap: output shape does not match order map shape"); + } + + const auto nrows = order_map.shape(0); + const auto ncols = order_map.shape(1); + + for (size_t row = 0; row < nrows; ++row) { + for (size_t col = 0; col < ncols; ++col) { + auto flat_index = order_map(row, col); + if (flat_index >= 0 && static_cast(flat_index) < input.size()) { - T const &value = input[flat_index]; - output(row, col) = value; - // output(row, col) = static_cast(input[flat_index]); + // T const &value = input[flat_index]; + // output(row, col) = value; + output(row, col) = input[flat_index]; } else { - output(row, col) = static_cast(0); // or nan? + // output(row, col) = static_cast(0); // or nan? + output(row, col) = T{}; } - } + } } } } // namespace aare::remap::algo \ No newline at end of file diff --git a/src/RemapAlgorithm.cpp b/src/RemapAlgorithm.cpp index 928f2e7..16a994d 100644 --- a/src/RemapAlgorithm.cpp +++ b/src/RemapAlgorithm.cpp @@ -64,11 +64,7 @@ strixel_to_pixel_map(defs::SensorGroupConfig const &group_config, // Define mod ordering (Normal or Inverse) std::vector mods(multiplicity); std::iota(mods.begin(), mods.end(), 0); - // This is a problem! Rotation does not invert the mods! - // (only one-axis mirroring does) - // -> Top-half of quad is only mirrored in y! - // if (rot == defs::Rotation::Inverse) - // std::reverse(mods.begin(), mods.end()); + if (group_config.routing.mod_order == defs::ColumnModOrdering::Reverse) std::reverse(mods.begin(), mods.end()); @@ -84,13 +80,6 @@ strixel_to_pixel_map(defs::SensorGroupConfig const &group_config, // -- 2) Apply transforms (if necessary) // -- 2a) bond_shift // -- 2b) rotation - // -- IMPORTANT: bond_shift BEFORE rotation! - // auto group_local = group_config; - // apply_rotation_shift(group_local, bond_shift, placement.rotation); - - // -- 2c) AFTER applying the transformations, we can grab the correct - // strixel roi - // auto roi_group = group_local.placement_on_sensor; InclusiveROI roi_group = shift_rotate_roi(group_config.placement_on_sensor, group_config.pixel, bond_shift, placement.rotation); @@ -143,11 +132,14 @@ strixel_to_pixel_map(defs::SensorGroupConfig const &group_config, return {multiplicity, pitch, eff, {}}; // nothing mapped } + // Now from the found bounds of the strixel grid, we define the space to + // allocate for the order map const int nrows_strx = max_row_strx - min_row_strx + 1; const int ncols_strx = max_col_strx - min_col_strx + 1; - // Allocate strixel grid order map + // And allocate aare::NDArray map({nrows_strx, ncols_strx}, -1); + // DEBUG std::cout << "DEBUG: Resulting strixel grid: (" << map.shape(0) << ", " << map.shape(1) << ")" << '\n'; @@ -189,19 +181,16 @@ strixel_to_pixel_map(defs::SensorGroupConfig const &group_config, std::vector strixel_to_pixel_maps(defs::SensorConfig const &sensor_config, - std::vector const &placements, + defs::SensorPlacement const &placement, InclusiveROI const &roi_user, defs::BondShift bond_shift) { - assert(sensor_config.group_configs.size() == placements.size()); - std::vector maps; maps.reserve(sensor_config.group_configs.size()); for (size_t i = 0; i < sensor_config.group_configs.size(); ++i) { - maps.emplace_back(strixel_to_pixel_map(sensor_config.group_configs[i], - placements[i], roi_user, - bond_shift)); + maps.emplace_back(strixel_to_pixel_map( + sensor_config.group_configs[i], placement, roi_user, bond_shift)); } return maps;