From 98cdc6ba08fc99ad618d339f1745f923a943bb83 Mon Sep 17 00:00:00 2001 From: vhinger Date: Wed, 22 Jul 2026 14:34:47 +0200 Subject: [PATCH] Refactoring and simplifications: - Adjust for reduced remap result struct - Add predefined SensorConfigs - Move combine_group_maps to implementation detail of RemapGenerate - Remove apply_rotation_shift and Rotation flip (have become superfluous) --- include/aare/RemapAlgorithm.hpp | 21 ---- include/aare/RemapConfig.hpp | 25 ++++- include/aare/RemapGenerate.hpp | 177 +++++++++++++++++++++++--------- src/RemapAlgorithm.cpp | 77 +------------- 4 files changed, 158 insertions(+), 142 deletions(-) diff --git a/include/aare/RemapAlgorithm.hpp b/include/aare/RemapAlgorithm.hpp index b41def82..013d82cf 100644 --- a/include/aare/RemapAlgorithm.hpp +++ b/include/aare/RemapAlgorithm.hpp @@ -4,23 +4,6 @@ namespace aare::remap::algo { -constexpr defs::Rotation flip(defs::Rotation r) noexcept { - switch (r) { - case defs::Rotation::Identity: - return defs::Rotation::Rotate180; - case defs::Rotation::Rotate180: - return defs::Rotation::Identity; - } - - assert(false && "Invalid Rotation passed to flip"); - return defs::Rotation::Identity; // Unreachable; satisfies compiler -} - -// Is it better to pass defs::GroupConfig const& and return a copy? -void apply_rotation_shift(defs::GroupConfig &, - defs::SensorPixelGeometry const &, defs::BondShift, - defs::Rotation); - defs::StrixelGroupToPixelMap strixel_to_pixel_map( defs::GroupConfig const &, defs::SensorPixelGeometry const &, defs::SensorModulePlacement const &, InclusiveROI const &user_roi, @@ -31,10 +14,6 @@ strixel_to_pixel_maps(defs::SensorConfig const &, defs::SensorModulePlacement co InclusiveROI const &user_roi, defs::BondShift bond_shift = {0, 0}); -defs::StrixelGroupToPixelMap -combine_maps(std::vector const &, - std::vector const &); - /** * Public API: * Applies a given remapping rule to an input array. diff --git a/include/aare/RemapConfig.hpp b/include/aare/RemapConfig.hpp index 3c2f92a2..f0104c5c 100644 --- a/include/aare/RemapConfig.hpp +++ b/include/aare/RemapConfig.hpp @@ -68,8 +68,15 @@ inline constexpr defs::GroupConfig SingleChipMP_iLGAD_P18{ * Number of strixel rows: 476 ********************************/ +// Complete SensorConfig with all groups +inline const defs::SensorConfig SingleChipMP_iLGAD{ + // NOTE: Vector cannot be constexpr + .pixel = SingleChipMP_iLGAD_pix, + .group_configs = {SingleChipMP_iLGAD_P25, SingleChipMP_iLGAD_P15, + SingleChipMP_iLGAD_P18}}; + /************************************ - * Single chip, multi-pitch, TEW + * Single chip, multi-pitch, Thin Entrance Window (TEW) ************************************/ inline constexpr defs::SensorPixelGeometry SingleChipMP_TEW_pix{ .num_pix_x = 256, .num_pix_y = 256, .guardring = {.x = 0, .y = 0}}; @@ -109,12 +116,21 @@ inline constexpr defs::GroupConfig SingleChipMP_TEW_P18{ * Number of strixel rows: 512 ********************************/ +// Complete SensorConfig with all groups +inline const defs::SensorConfig SingleChipMP_TEW{ + // NOTE: Vector cannot be constexpr + .pixel = SingleChipMP_TEW_pix, + .group_configs = {SingleChipMP_TEW_P25, SingleChipMP_TEW_P15, + SingleChipMP_TEW_P18}}; + /************************************ * Quad, 25 um, iLGAD ************************************/ inline constexpr defs::SensorPixelGeometry Quad_iLGAD_pix{ .num_pix_x = 512, .num_pix_y = 512, .guardring = {.x = 9, .y = 9}}; +inline constexpr int Quad_iLGAD_strixel_gap_rows = 12; + inline constexpr defs::GroupConfig Quad_iLGAD_bottomhalf{ .strixel = StrxP25, .placement_on_sensor = {Quad_iLGAD_pix.guardring.x + 2, // 11 @@ -126,10 +142,15 @@ inline constexpr defs::GroupConfig Quad_iLGAD_bottomhalf{ inline constexpr defs::GroupConfig Quad_iLGAD_tophalf{ .strixel = StrxP25, .routing = {defs::ModuloOrdering::Reverse}, - // Adapt placement to be correct! .placement_on_sensor = { Quad_iLGAD_pix.guardring.x + 2, // 11 Quad_iLGAD_pix.num_pix_x - Quad_iLGAD_pix.guardring.x - 1, // 502 Quad_iLGAD_pix.num_pix_y / 2 + 1, // 257 Quad_iLGAD_pix.num_pix_y - Quad_iLGAD_pix.guardring.y - 1}}; // 502 + +// Complete SensorConfig with all groups +inline const defs::SensorConfig Quad_iLGAD{ + // NOTE: Vector cannot be constexpr + .pixel = Quad_iLGAD_pix, + .group_configs = {Quad_iLGAD_bottomhalf, Quad_iLGAD_tophalf}}; } // namespace aare::remap::config::jungfrau \ No newline at end of file diff --git a/include/aare/RemapGenerate.hpp b/include/aare/RemapGenerate.hpp index 71877f8c..893e0c82 100644 --- a/include/aare/RemapGenerate.hpp +++ b/include/aare/RemapGenerate.hpp @@ -5,33 +5,134 @@ namespace aare::remap::generate { +// ============================================================ +// Internal implementation details +// ============================================================ +// Current quick-and-dirty inclusion of a helper function to combine group maps. +// (Used to live as `combine_maps` in RemapAlgorithm, but arguably should not be +// part of that public API). Eventually, one could instead split RemapGenerate +// into hpp and cpp and make this one live in an unnamed namespace inside the +// cpp so that it cannot be exposed to the user through the public API (if we +// want to avoid that) +namespace detail { +/** + * @brief Combine TWO vertically ordered group remapping maps. + * + * Concatenates the maps in the order provided, inserting `gap_rows` + * rows of invalid entries (-1) between consecutive groups. + * + * The input maps must be ordered in the desired output order. + * Both maps must have the same number of columns. + * + * The returned effective ROI is the bounding ROI covering the + * effective ROIs of both input groups. It describes the physical + * source-pixel region and does not encode the artificial strixel + * gap rows in the output map. + * + * @param first Group map that comes first in output space. + * @param second Group map that comes second in output space. + * @param gap_rows Number of invalid strixel rows inserted between groups. + * + * @return Combined strixel-to-pixel map. + * + * @throws std::invalid_argument if gap_rows is negative. + * @throws std::logic_error if group maps have different widths. + */ +defs::StrixelGroupToPixelMap +combine_group_maps(defs::StrixelGroupToPixelMap const &first, + defs::StrixelGroupToPixelMap const &second, int gap_rows) { + + if (gap_rows < 0) { + throw std::invalid_argument("gap_rows must be non-negative"); + } + + const ssize_t ncols = first.map.shape(1); + + // Make sure both maps have the same width. + if (second.map.shape(1) != ncols) { + throw std::logic_error("Cannot combine maps with different numbers " + "of columns"); + } + + // Check effective ROIs line up + if (first.effective_roi.xmin != second.effective_roi.xmin || + first.effective_roi.xmax != second.effective_roi.xmax) { + throw std::logic_error( + "Cannot combine group maps with different x extents"); + } + + // Calculate total number of output rows. + ssize_t total_rows = 0; + total_rows = first.map.shape(0) + second.map.shape(0) + + static_cast(gap_rows); + + // Allocate and initialize with -1. + // + // -1 represents an output strixel position that has + // no corresponding input pixel. + NDArray combined({total_rows, ncols}, -1); + + // Copy maps into the combined output. + auto copy_map = [&](auto const &source, ssize_t destination_row) { + const ssize_t nrows = source.map.shape(0); + + for (ssize_t row = 0; row < nrows; ++row) { + for (ssize_t col = 0; col < ncols; ++col) { + combined(destination_row + row, col) = source.map(row, col); + } + } + }; + + const ssize_t first_row = 0; + const ssize_t second_row = first.map.shape(0) + gap_rows; + + copy_map(first, first_row); + copy_map(second, second_row); + + // Physical pixel ROI covered by both groups. + InclusiveROI effective_roi = first.effective_roi; + + effective_roi.xmin = + std::min(effective_roi.xmin, second.effective_roi.xmin); + effective_roi.xmax = + std::max(effective_roi.xmax, second.effective_roi.xmax); + effective_roi.ymin = + std::min(effective_roi.ymin, second.effective_roi.ymin); + effective_roi.ymax = + std::max(effective_roi.ymax, second.effective_roi.ymax); + + return {.map = std::move(combined), .effective_roi = effective_roi}; +} +} // namespace detail + +// ============================================================ +// Public generators +// ============================================================ + /************************************ * Single chip, multi-pitch, iLGAD ************************************/ -inline defs::StrixelGroupToPixelMap -jungfrau_ilgad_singlechip_25um_strixel_map(InclusiveROI rx_roi, - defs::SensorModulePlacement placement, - defs::BondShift bs = {0, 0}) { +inline defs::StrixelGroupToPixelMap jungfrau_ilgad_singlechip_25um_strixel_map( + InclusiveROI rx_roi, defs::SensorModulePlacement placement, + defs::BondShift bs = {0, 0}) { std::cout << " === JUNGFRAU iLGAD SINGLE-CHIP 25um PITCH === \n"; return algo::strixel_to_pixel_map(config::jungfrau::SingleChipMP_iLGAD_P25, config::jungfrau::SingleChipMP_iLGAD_pix, placement, rx_roi, bs); } -inline defs::StrixelGroupToPixelMap -jungfrau_ilgad_singlechip_15um_strixel_map(InclusiveROI rx_roi, - defs::SensorModulePlacement placement, - defs::BondShift bs = {0, 0}) { +inline defs::StrixelGroupToPixelMap jungfrau_ilgad_singlechip_15um_strixel_map( + InclusiveROI rx_roi, defs::SensorModulePlacement placement, + defs::BondShift bs = {0, 0}) { std::cout << " === JUNGFRAU iLGAD SINGLE-CHIP 15um PITCH === \n"; return algo::strixel_to_pixel_map(config::jungfrau::SingleChipMP_iLGAD_P15, config::jungfrau::SingleChipMP_iLGAD_pix, placement, rx_roi, bs); }; -inline defs::StrixelGroupToPixelMap -jungfrau_ilgad_singlechip_18um_strixel_map(InclusiveROI rx_roi, - defs::SensorModulePlacement placement, - defs::BondShift bs = {0, 0}) { +inline defs::StrixelGroupToPixelMap jungfrau_ilgad_singlechip_18um_strixel_map( + InclusiveROI rx_roi, defs::SensorModulePlacement placement, + defs::BondShift bs = {0, 0}) { std::cout << " === JUNGFRAU iLGAD SINGLE-CHIP 18.75um PITCH === \n"; return algo::strixel_to_pixel_map(config::jungfrau::SingleChipMP_iLGAD_P18, config::jungfrau::SingleChipMP_iLGAD_pix, @@ -54,12 +155,9 @@ jungfrau_ilgad_singlechip_multipitch_strixel_maps(InclusiveROI rx_roi, // std::optional sensor_placement) throw std::runtime_error("Invalid sensor placement."); } - defs::SensorConfig configs{config::jungfrau::SingleChipMP_iLGAD_pix, - {config::jungfrau::SingleChipMP_iLGAD_P25, - config::jungfrau::SingleChipMP_iLGAD_P15, - config::jungfrau::SingleChipMP_iLGAD_P18}}; - return algo::strixel_to_pixel_maps(configs, placement, rx_roi, bs); + return algo::strixel_to_pixel_maps(config::jungfrau::SingleChipMP_iLGAD, + placement, rx_roi, bs); }; // More generic overload @@ -67,12 +165,9 @@ inline std::vector jungfrau_ilgad_singlechip_multipitch_strixel_maps( InclusiveROI rx_roi, defs::SensorModulePlacement placement, defs::BondShift bs = {0, 0}) { - defs::SensorConfig configs{config::jungfrau::SingleChipMP_iLGAD_pix, - {config::jungfrau::SingleChipMP_iLGAD_P25, - config::jungfrau::SingleChipMP_iLGAD_P15, - config::jungfrau::SingleChipMP_iLGAD_P18}}; - return algo::strixel_to_pixel_maps(configs, placement, rx_roi, bs); + return algo::strixel_to_pixel_maps(config::jungfrau::SingleChipMP_iLGAD, + placement, rx_roi, bs); }; /************************************ @@ -124,33 +219,26 @@ jungfrau_tew_singlechip_multipitch_strixel_maps(InclusiveROI rx_roi, // std::optional sensor_placement) throw std::runtime_error("Invalid sensor placement."); } - defs::SensorConfig configs{config::jungfrau::SingleChipMP_TEW_pix, - {config::jungfrau::SingleChipMP_TEW_P25, - config::jungfrau::SingleChipMP_TEW_P15, - config::jungfrau::SingleChipMP_TEW_P18}}; - return algo::strixel_to_pixel_maps(configs, placement, rx_roi, bs); + return algo::strixel_to_pixel_maps(config::jungfrau::SingleChipMP_TEW, + placement, rx_roi, bs); }; // More generic overload inline std::vector -jungfrau_tew_singlechip_multipitch_strixel_maps(InclusiveROI rx_roi, - defs::SensorModulePlacement placement, - defs::BondShift bs = {0, 0}) { - defs::SensorConfig configs{config::jungfrau::SingleChipMP_TEW_pix, - {config::jungfrau::SingleChipMP_TEW_P25, - config::jungfrau::SingleChipMP_TEW_P15, - config::jungfrau::SingleChipMP_TEW_P18}}; +jungfrau_tew_singlechip_multipitch_strixel_maps( + InclusiveROI rx_roi, defs::SensorModulePlacement placement, + defs::BondShift bs = {0, 0}) { - return algo::strixel_to_pixel_maps(configs, placement, rx_roi, bs); + return algo::strixel_to_pixel_maps(config::jungfrau::SingleChipMP_TEW, + placement, rx_roi, bs); }; /************************************ * Quad, 25 um, iLGAD ************************************/ -inline defs::StrixelGroupToPixelMap -jungfrau_ilgad_quadbottom_25um_strixel_map(InclusiveROI rx_roi, - defs::SensorModulePlacement placement) { +inline defs::StrixelGroupToPixelMap jungfrau_ilgad_quadbottom_25um_strixel_map( + InclusiveROI rx_roi, defs::SensorModulePlacement placement) { return algo::strixel_to_pixel_map(config::jungfrau::Quad_iLGAD_bottomhalf, config::jungfrau::Quad_iLGAD_pix, placement, rx_roi); @@ -168,19 +256,16 @@ inline std::vector jungfrau_ilgad_quad_25um_strixel_maps(InclusiveROI rx_roi, defs::SensorModulePlacement placement) { - defs::SensorConfig configs{config::jungfrau::Quad_iLGAD_pix, - {config::jungfrau::Quad_iLGAD_bottomhalf, - config::jungfrau::Quad_iLGAD_tophalf}}; - - return algo::strixel_to_pixel_maps(configs, placement, rx_roi); + return algo::strixel_to_pixel_maps(config::jungfrau::Quad_iLGAD, placement, + rx_roi); } inline defs::StrixelGroupToPixelMap jungfrau_ilgad_quad_25um_strixel_map(InclusiveROI rx_roi, defs::SensorModulePlacement placement, defs::BondShift bs = {0, 0}) { - std::vector gap_rows{12, 0}; auto maps = jungfrau_ilgad_quad_25um_strixel_maps(rx_roi, placement); - return algo::combine_maps(maps, gap_rows); + return detail::combine_group_maps( + maps[0], maps[1], config::jungfrau::Quad_iLGAD_strixel_gap_rows); } -} // namespace aare::remap::generate \ No newline at end of file +} // namespace aare::remap::generate diff --git a/src/RemapAlgorithm.cpp b/src/RemapAlgorithm.cpp index 95fd887f..d1c98a04 100644 --- a/src/RemapAlgorithm.cpp +++ b/src/RemapAlgorithm.cpp @@ -4,23 +4,8 @@ namespace aare::remap::algo { -// Is it better to pass defs::SensorGroupConfig const& and return a copy? -// Better not to, use shift_rotate_roi instead -void apply_rotation_shift(defs::GroupConfig &cfg, - defs::SensorPixelGeometry const &pixel, - defs::BondShift bond_shift, defs::Rotation rot) { - // Apply physical transforms - if (bond_shift.x != 0 || bond_shift.y != 0) - cfg.placement_on_sensor = aare::inclusiveroi::geom::translate( - cfg.placement_on_sensor, bond_shift.x, bond_shift.y); - - if (rot == defs::Rotation::Rotate180) - cfg.placement_on_sensor = aare::inclusiveroi::geom::mirrorXY( - cfg.placement_on_sensor, pixel.num_pix_x, pixel.num_pix_y); -} - /** - * Apply physical transformations to a sensor-local ROI. + * @brief Apply physical transformations to a sensor-local ROI. * * IMPORTANT: * Bond shifts are applied before rotation. @@ -94,7 +79,7 @@ strixel_to_pixel_map(defs::GroupConfig const &group_config, // -- 3) Compute effective ROI = intersection( roi_user, roi_group ) InclusiveROI eff = inclusiveroi::geom::intersect(roi_user_local, roi_group); if (eff.xmax < eff.xmin || eff.ymax < eff.ymin) { - return {-1, 0.0, InclusiveROI::emptyROI(), {}}; // empty + return {{}, InclusiveROI::emptyROI()}; // empty } // DEBUG @@ -131,7 +116,7 @@ strixel_to_pixel_map(defs::GroupConfig const &group_config, } if (min_row_strx > max_row_strx) { - return {multiplicity, pitch, eff, {}}; // nothing mapped + return {{}, eff}; // nothing mapped } // Now from the found bounds of the strixel grid, we define the space to @@ -178,7 +163,7 @@ strixel_to_pixel_map(defs::GroupConfig const &group_config, } } - return {multiplicity, pitch, eff, map}; + return {map, eff}; }; std::vector @@ -199,58 +184,4 @@ strixel_to_pixel_maps(defs::SensorConfig const &sensor_config, return maps; } -defs::StrixelGroupToPixelMap -combine_maps(std::vector const &maps, - std::vector const &gaps) { - - if (maps.size() != gaps.size()) { - throw std::logic_error("Gaps provided are inclompatible with " - "number of maps to combine. Number of gaps must " - "be equal the number of maps."); - } - - auto [_, global_cols] = maps[0].map.shape(); - int global_rows = 0; - int const m = maps[0].multiplicity; - double const p = maps[0].pitch_um; - auto placement_on_sensor = maps[0].placement_on_sensor; - std::vector offsets(maps.size()); - for (size_t i = 0; i < maps.size(); ++i) { - - offsets[i] = global_rows; - - if (maps[i].multiplicity != m) { - throw std::logic_error("Maps contain incompatible multiplicities."); - } - - if (maps[i].pitch_um != p) { - throw std::logic_error("Maps contain incompatible pitches."); - } - - auto [temp_rows, temp_cols] = maps[i].map.shape(); - global_cols = std::max(global_cols, temp_cols); - global_rows = global_rows + temp_rows + gaps[i]; - } - - NDArray map({global_rows, global_cols}, -1); - - for (size_t i = 0; i < maps.size(); ++i) { - auto [rows, cols] = maps[i].map.shape(); - - // DEBUG - std::cout << "DEBUG: Row offset: i = " << i - << ", offset: " << offsets[i] << '\n'; - - for (ssize_t r = 0; r < rows; ++r) { - for (ssize_t c = 0; c < cols; ++c) { - map(offsets[i] + r, c) = maps[i].map(r, c); - } - } - } - - // For combined maps, placement_on_sensor in principle is no longer correct - // TODO: Decide how to handle this! - return {m, p, placement_on_sensor, map}; -} - } // namespace aare::remap::algo \ No newline at end of file