diff --git a/RELEASE.md b/RELEASE.md index 3bf40b65..fcfb76ad 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -7,8 +7,9 @@ - Added gap pixel insertion for Jungfrau and Eiger raw files. ``RawFile``, ``File`` (through ``FileConfig::gap_pixels``) and ``MultiThreadedFileReader`` take a ``GapPixels`` configuration, in Python - the ``gap_pixels`` argument, and assemble frames with 2 pixel chip gaps - and optional ``ModuleGaps`` between modules. Gap positions follow + the ``gap_pixels`` argument, and assemble frames with 2 pixel gaps at + the chip boundaries inside each module and, only when ``ModuleGaps`` is + set, gaps between modules. Gap positions follow detector coordinates before any ROI. Gap pixels receive a fill value or, with ``split_counts``, half of the neighbouring double size pixel. The new ``insert_gap_pixels`` and ``gapped_shape`` apply the same transform diff --git a/docs/src/GapPixels.rst b/docs/src/GapPixels.rst index 9e49d075..34a2acce 100644 --- a/docs/src/GapPixels.rst +++ b/docs/src/GapPixels.rst @@ -4,8 +4,8 @@ GapPixels Jungfrau and Eiger modules are built from 256x256 pixel chips. The pixels at a chip edge are physically double size, so an image that places the chips next to each other is distorted. Gap pixels restore the geometry by -inserting pixels at every chip boundary and, optionally, wider gaps -between modules. +inserting pixels at every chip boundary inside a module and, optionally, +gaps between modules. Gap positions are determined in detector coordinates before any ROI. A ROI only receives the gaps that fall strictly inside it, so a ROI that @@ -26,9 +26,9 @@ Field Default Meaning inside a module. Source column 255 expands to the right and column 256 to the left. ``module_gaps`` unset ``ModuleGaps{x, y}`` inserted at module - boundaries instead of the chip gap. Unset - means module boundaries receive the chip - gap. ``ModuleGaps{8, 36}`` matches the + boundaries. Unset means no pixels are + inserted between modules. + ``ModuleGaps{8, 36}`` matches the slsDetectorPackage GUI. ``fill_value`` 0 Value written to gap pixels, converted to the pixel type. Out of range values wrap @@ -47,8 +47,8 @@ Image sizes with the default configuration and with ``ModuleGaps{8, 36}``: Detector range Ungapped Defaults ``ModuleGaps{8,36}`` ====================================== ========== ========= =================== One module, columns 1024 1030 1030 -Two modules side by side, columns 2048 2062 2068 -Two modules stacked, rows 1024 1030 1064 +Two modules side by side, columns 2048 2060 2068 +Two modules stacked, rows 1024 1028 1064 ROI columns 100 to 400 300 302 302 ROI columns 256 to 512 256 256 256 ====================================== ========== ========= =================== diff --git a/docs/src/python/file/pyGapPixels.rst b/docs/src/python/file/pyGapPixels.rst index 73e36ecf..862c8d72 100644 --- a/docs/src/python/file/pyGapPixels.rst +++ b/docs/src/python/file/pyGapPixels.rst @@ -5,7 +5,8 @@ GapPixels Jungfrau and Eiger modules are built from 256x256 pixel chips with double size pixels at the chip edges. Gap pixels restore the geometry by inserting -pixels at every chip boundary and, optionally, wider gaps between modules. +pixels at every chip boundary inside a module and, optionally, gaps between +modules. Gap positions are determined in detector coordinates before any ROI, so a ROI only receives the gaps that fall strictly inside it. @@ -19,9 +20,9 @@ Field Default Meaning ``chip_gap`` 2 Pixels inserted at each chip boundary inside a module. ``module_gaps`` None ``ModuleGaps(x, y)`` inserted at module - boundaries instead of the chip gap. None - means module boundaries receive the chip - gap. ``ModuleGaps(8, 36)`` matches the + boundaries. None means no pixels are + inserted between modules. + ``ModuleGaps(8, 36)`` matches the slsDetectorPackage GUI. ``fill_value`` 0 Value written to gap pixels, converted to the pixel dtype. Module gaps always @@ -34,7 +35,7 @@ Field Default Meaning ================ ================ =========================================== A single Jungfrau module reads as ``(514, 1030)`` with the defaults. Two -stacked modules read as ``(1030, 1030)`` with the defaults and +stacked modules read as ``(1028, 1030)`` with the defaults and ``(1064, 1030)`` with ``ModuleGaps(8, 36)``. .. code-block:: python diff --git a/include/aare/GapPixels.hpp b/include/aare/GapPixels.hpp index b9e831e5..f187cf78 100644 --- a/include/aare/GapPixels.hpp +++ b/include/aare/GapPixels.hpp @@ -18,8 +18,7 @@ namespace aare { /** - * @brief Pixels inserted at each boundary between modules, replacing the - * chip gap there. + * @brief Pixels inserted at each boundary between modules. */ struct ModuleGaps { ssize_t x{}; @@ -34,7 +33,9 @@ struct ModuleGaps { /** * @brief Configuration for inserting gap pixels into Jungfrau and Eiger * images. Chip and module sizes are fixed by the detector type: 256x256 - * pixel chips in 1024x512 pixel modules (512x512 for an Eiger quad). + * pixel chips in 1024x512 pixel modules (512x512 for an Eiger quad). Gaps + * are inserted at the chip boundaries inside each module and, only when + * module_gaps is set, between modules. * * Gap positions are determined in detector coordinates before any ROI, so * a ROI only receives the gaps that fall strictly inside it. @@ -44,8 +45,8 @@ struct GapPixels { /// column 255 expands to the right and column 256 to the left. ssize_t chip_gap{2}; - /// @brief Pixels inserted at module boundaries instead of the chip gap. - /// Unset means module boundaries receive the chip gap. + /// @brief Pixels inserted at module boundaries. Unset means no pixels + /// are inserted between modules. std::optional module_gaps{}; /// @brief Value written to gap pixels, converted to the pixel type. diff --git a/python/src/bind_GapPixels.hpp b/python/src/bind_GapPixels.hpp index b8af904f..0fc19bcb 100644 --- a/python/src/bind_GapPixels.hpp +++ b/python/src/bind_GapPixels.hpp @@ -32,8 +32,7 @@ py::array insert_gap_pixels_typed(const py::array &image, const ROI &roi, void define_gap_pixels_bindings(py::module &m) { py::class_(m, "ModuleGaps", R"( - Pixels inserted at each boundary between modules, replacing the chip - gap there. + Pixels inserted at each boundary between modules. )") .def(py::init<>()) .def(py::init(), py::arg("x"), py::arg("y")) @@ -55,8 +54,8 @@ void define_gap_pixels_bindings(py::module &m) { Pixels inserted at each chip boundary inside a module. Source column 255 expands to the right and column 256 to the left. module_gaps : ModuleGaps, optional - Pixels inserted at module boundaries instead of the chip gap. - None means module boundaries receive the chip gap. + Pixels inserted at module boundaries. None means no pixels are + inserted between modules. fill_value : float Value written to gap pixels, converted to the pixel type. Module gaps always receive this value. diff --git a/python/tests/test_GapPixels.py b/python/tests/test_GapPixels.py index 0eb8e03d..147b9947 100644 --- a/python/tests/test_GapPixels.py +++ b/python/tests/test_GapPixels.py @@ -34,7 +34,7 @@ def test_configuration_defaults_and_equality(): def test_gapped_shape(): assert gapped_shape(ROI(0, 1024, 0, 512), DetectorType.Jungfrau) == (514, 1030) - assert gapped_shape(ROI(0, 1024, 0, 1024), DetectorType.Jungfrau) == (1030, 1030) + assert gapped_shape(ROI(0, 1024, 0, 1024), DetectorType.Jungfrau) == (1028, 1030) assert gapped_shape(ROI(0, 1024, 0, 1024), DetectorType.Jungfrau, GapPixels(module_gaps=ModuleGaps(8, 36))) == (1064, 1030) assert gapped_shape(ROI(0, 512, 0, 512), DetectorType.Eiger, quad=True) == (514, 514) assert gapped_shape(ROI(256, 512, 0, 256), DetectorType.Jungfrau) == (256, 256) @@ -54,6 +54,16 @@ def test_insert_gap_pixels_matches_numpy(dtype): assert (result == expected).all() +def test_insert_gap_pixels_without_module_gaps(): + # two stacked modules: chip gaps at rows 256 and 768, none at the module + # boundary at row 512 + image = np.arange(1024 * 1024, dtype=np.uint32).reshape(1024, 1024) + result = insert_gap_pixels(image, DetectorType.Jungfrau) + expected = reference(image, [(256, 2), (512, 2), (768, 2)], [(256, 2), (768, 2)]) + assert result.shape == (1028, 1030) + assert (result == expected).all() + + def test_insert_gap_pixels_with_module_gaps(): image = np.arange(1024 * 1024, dtype=np.uint32).reshape(1024, 1024) gaps = GapPixels(module_gaps=ModuleGaps(8, 36), fill_value=9) diff --git a/python/tests/test_RawFile.py b/python/tests/test_RawFile.py index 85353393..d421ac34 100644 --- a/python/tests/test_RawFile.py +++ b/python/tests/test_RawFile.py @@ -527,8 +527,9 @@ def test_raw_file_gap_pixels(tmp_path, layout): gaps = GapPixels(fill_value=5) with RawFile(master_path, gap_pixels=gaps) as reader: assert reader.gap_pixels == gaps - assert (reader.rows(), reader.cols()) == (512 * layout_rows + 2 * (layout_rows * 2 - 1), - 1024 * layout_cols + 2 * (layout_cols * 4 - 1)) + # one chip boundary per module row, three per module column and no + # gap between modules + assert (reader.rows(), reader.cols()) == ((512 + 2) * layout_rows, (1024 + 6) * layout_cols) header, image = reader.read_frame() assert header.size == layout_rows * layout_cols assert (image == insert_gap_pixels(expected, DetectorType.Jungfrau, gaps)).all() diff --git a/src/GapPixels.cpp b/src/GapPixels.cpp index 38f94aa5..3fb6c7e0 100644 --- a/src/GapPixels.cpp +++ b/src/GapPixels.cpp @@ -15,8 +15,7 @@ GapLayout gap_layout(DetectorType detector, bool quad, const GapPixels &gaps) { if (gaps.chip_gap < 0) { throw std::invalid_argument(LOCATION + "Chip gap must not be negative"); } - const ModuleGaps module_gaps = - gaps.module_gaps.value_or(ModuleGaps{gaps.chip_gap, gaps.chip_gap}); + const ModuleGaps module_gaps = gaps.module_gaps.value_or(ModuleGaps{}); if (module_gaps.x < 0 || module_gaps.y < 0) { throw std::invalid_argument(LOCATION + "Module gaps must not be negative"); diff --git a/src/GapPixels.test.cpp b/src/GapPixels.test.cpp index 8ccfebd5..b1859924 100644 --- a/src/GapPixels.test.cpp +++ b/src/GapPixels.test.cpp @@ -76,11 +76,11 @@ TEST_CASE("Gap layout follows the detector type", "[GapPixels]") { CHECK(layout.x.chip == 256); CHECK(layout.x.module == 1024); CHECK(layout.x.chip_gap == 2); - CHECK(layout.x.module_gap == 2); + CHECK(layout.x.module_gap == 0); CHECK(layout.y.chip == 256); CHECK(layout.y.module == 512); CHECK(layout.y.chip_gap == 2); - CHECK(layout.y.module_gap == 2); + CHECK(layout.y.module_gap == 0); } SECTION("Eiger quad has a 512 pixel wide module") { CHECK(detail::gap_layout(DetectorType::Eiger, true, GapPixels{}) @@ -135,11 +135,11 @@ TEST_CASE("Gapped shape of detector ROIs", "[GapPixels]") { CHECK(gapped_shape(ROI{0, 1024, 0, 512}, DetectorType::Jungfrau, false, module_8_36) == Shape{514, 1030}); CHECK(gapped_shape(ROI{0, 2048, 0, 512}, DetectorType::Jungfrau, false) == - Shape{514, 2062}); + Shape{514, 2060}); CHECK(gapped_shape(ROI{0, 2048, 0, 512}, DetectorType::Jungfrau, false, module_8_36) == Shape{514, 2068}); CHECK(gapped_shape(ROI{0, 1024, 0, 1024}, DetectorType::Jungfrau, false) == - Shape{1030, 1030}); + Shape{1028, 1030}); CHECK(gapped_shape(ROI{0, 1024, 0, 1024}, DetectorType::Jungfrau, false, module_8_36) == Shape{1064, 1030}); CHECK(gapped_shape(ROI{0, 512, 0, 512}, DetectorType::Eiger, true) == @@ -155,7 +155,7 @@ TEST_CASE("Gapped shape of detector ROIs", "[GapPixels]") { CHECK(gapped_shape(ROI{255, 257, 0, 1}, DetectorType::Jungfrau, false) == Shape{1, 4}); CHECK(gapped_shape(ROI{200, 301, 300, 601}, DetectorType::Jungfrau, - false) == Shape{303, 103}); + false) == Shape{301, 103}); CHECK(gapped_shape(ROI{200, 301, 300, 601}, DetectorType::Jungfrau, false, module_8_36) == Shape{337, 103}); CHECK(gapped_shape(ROI{512, 768, 0, 256}, DetectorType::Jungfrau, @@ -369,6 +369,35 @@ TEST_CASE("insert_gap_pixels on ROIs", "[GapPixels]") { } } +TEST_CASE("Module boundaries receive no gap unless module gaps are set", + "[GapPixels]") { + // rows 510..513 of two stacked modules hold the module boundary at 512 + const ROI roi{0, 2, 510, 514}; + auto source = pattern(4, 2); + GapPixels gaps; + gaps.fill_value = 9; + SECTION("fill value") {} + SECTION("split counts") { + gaps.split_counts = true; + gaps.seed = 1; + } + auto result = insert_gap_pixels(NDView(source.view()), + roi, DetectorType::Jungfrau, false, gaps); + REQUIRE(result.shape() == std::array{4, 2}); + CHECK(result == source); + + gaps.module_gaps = ModuleGaps{0, 3}; + auto with_gap = insert_gap_pixels(NDView(source.view()), + roi, DetectorType::Jungfrau, false, gaps); + REQUIRE(with_gap.shape() == std::array{7, 2}); + for (ssize_t row = 2; row < 5; ++row) { + CHECK(with_gap(row, 0) == 9); + CHECK(with_gap(row, 1) == 9); + } + CHECK(with_gap(1, 0) == source(1, 0)); + CHECK(with_gap(5, 0) == source(2, 0)); +} + TEST_CASE("Split counts between edge pixels and their gaps", "[GapPixels]") { GapPixels gaps; gaps.split_counts = true; diff --git a/src/RawFile.test.cpp b/src/RawFile.test.cpp index f4dc29e0..d6ebf744 100644 --- a/src/RawFile.test.cpp +++ b/src/RawFile.test.cpp @@ -476,12 +476,12 @@ TEST_CASE("Gap pixels on recorded Jungfrau and Eiger files", {514, 514}, {}}, {"raw/virtual/920/jungfrau/two_modules_master_0.json", - {1030, 1030}, + {1028, 1030}, {}}, {"raw/virtual/920/jungfrau/two_modules_master_0.json", {1064, 1030}, ModuleGaps{8, 36}}, - {"raw/ROITestData/MultipleChipROI/run_master_2.json", {303, 103}, {}}, + {"raw/ROITestData/MultipleChipROI/run_master_2.json", {301, 103}, {}}, {"raw/ROITestData/MultipleChipROI/run_master_2.json", {337, 103}, ModuleGaps{8, 36}},