From 842c43a86e12f2abd5b368e6ff1993f3afd1e217 Mon Sep 17 00:00:00 2001 From: leonarski_f Date: Sat, 22 Aug 2026 23:07:32 +0200 Subject: [PATCH] Send the sample transformation chain in mounting order The chain could not be expressed. A goniometer axis, the Smargon chi/phi and a grid stage each travelled by a different route - the `goniometer` map, a private JSON key inside user_data, and `grid_scan` - and nothing said what order they are mounted in. The order cannot go in the `goniometer` map either: that is a DECTRIS stream2 field, and RFC 8949 requires deterministic encoders to SORT map keys, so a map's order is not something a consumer may rely on. So `transformations` is sent as an ordered ARRAY, base first, each element carrying name, type, axis vector and angles. The `goniometer` map and `grid_scan` are still emitted beside it, unchanged, for consumers that only know stream2 - nothing vendor-defined is mutated, and a stream2 consumer sees exactly what it saw before. Smargon chi and phi are ordinary stationary axes that were only ever separate for historical reasons, and they now appear in the chain like any other. They are also read back: reader/ had no smargon support at all, so re-opening a file lost the head position silently. Combined with the earlier change that writes them for a still rather than only alongside a rotation or a grid scan, the round trip is now closed. Metadata version 7. A broker and a writer from different releases must not be mixed across this: an older writer ignores the chain and reads the unordered map, so anything whose order matters - a Smargon position, or a grid scan combined with a rotation - is not reproduced. Said so in docs/CBOR.md and the changelog. CBORSerialize_Start_Transformations asserts the ORDER survives, not just the contents, which is the whole point of the array. Co-Authored-By: Claude Opus 5 (1M context) --- common/DiffractionExperiment.cpp | 36 +++++++++++++++++++ common/DiffractionExperiment.h | 3 ++ common/JFJochMessages.h | 22 +++++++++++- docs/CBOR.md | 7 ++++ docs/CHANGELOG.md | 4 +++ frame_serialize/CBORStream2Deserializer.cpp | 39 +++++++++++++++++++++ frame_serialize/CBORStream2Serializer.cpp | 27 +++++++++++++- reader/HDF5MetadataSource.cpp | 18 ++++++++++ tests/CBORTest.cpp | 34 ++++++++++++++++++ 9 files changed, 188 insertions(+), 2 deletions(-) diff --git a/common/DiffractionExperiment.cpp b/common/DiffractionExperiment.cpp index 49eb1d57..a004a1c5 100644 --- a/common/DiffractionExperiment.cpp +++ b/common/DiffractionExperiment.cpp @@ -717,6 +717,7 @@ void DiffractionExperiment::FillMessage(StartMessage &message) const { message.goniometer = dataset.GetGoniometer(); message.grid_scan = dataset.GetGridScan(); + message.transformations = BuildTransformationChain(); message.run_number = GetRunNumber(); message.run_name = GetRunName(); @@ -915,6 +916,11 @@ DiffractionExperiment &DiffractionExperiment::Goniometer(const std::optional &input) { + dataset.Smargon(input); + return *this; +} + std::optional DiffractionExperiment::GetGoniometer() const { return dataset.GetGoniometer(); } @@ -1450,6 +1456,36 @@ bool DiffractionExperiment::IsDetectorMirroredY() const { return detector.IsMirrorY(); } +// Base first, sample last - the order things are physically mounted in, which is also the order an +// NXmx depends_on chain has to be built in. The grid stage is a base stage (an Aerotech xyz at SLS) +// that the spindle sits on; the spindle carries the head; the head carries the sample. +std::vector DiffractionExperiment::BuildTransformationChain() const { + std::vector chain; + + if (const auto grid_scan = GetGridScan()) { + chain.push_back({.name = "grid_scan_x", .rotation = false, .vector = {1, 0, 0}}); + chain.push_back({.name = "grid_scan_y", .rotation = false, .vector = {0, 1, 0}}); + } + + if (const auto goniometer = GetGoniometer()) + chain.push_back({.name = goniometer->GetName(), + .rotation = true, + .vector = goniometer->GetAxis(), + .start = goniometer->GetStart_deg(), + .increment = goniometer->GetIncrement_deg()}); + + // Smargon chi and phi are ordinary axes that happen not to move; they are only separate in the + // settings for historical reasons. + if (const auto smargon = dataset.GetSmargonPosition()) { + chain.push_back({.name = "chi", .rotation = true, + .vector = smargon->chi_axis, .start = smargon->chi_deg}); + chain.push_back({.name = "phi", .rotation = true, + .vector = smargon->phi_axis, .start = smargon->phi_deg}); + } + + return chain; +} + int64_t DiffractionExperiment::GetEigerBitDepth() const { auto tmp = detector_settings.GetEigerBitDepth(); if (tmp.has_value()) diff --git a/common/DiffractionExperiment.h b/common/DiffractionExperiment.h index 2aee034b..9e7d06e1 100644 --- a/common/DiffractionExperiment.h +++ b/common/DiffractionExperiment.h @@ -133,6 +133,7 @@ public: DiffractionExperiment& AttenuatorTransmission(const std::optional &input); DiffractionExperiment& TotalFlux(const std::optional &input); DiffractionExperiment& Goniometer(const std::optional &input); + DiffractionExperiment& Smargon(const std::optional &input); DiffractionExperiment& HeaderAppendix(const nlohmann::json& input); DiffractionExperiment& ImageAppendix(const nlohmann::json& input); DiffractionExperiment& Summation(int64_t input); @@ -390,6 +391,8 @@ public: bool IsDetectorModuleSync() const; bool IsDetectorMirroredY() const; + // The sample transformation chain in mounting order, base first. See TransformationAxis. + [[nodiscard]] std::vector BuildTransformationChain() const; [[nodiscard]] DetectorType GetDetectorType() const; [[nodiscard]] bool IsMaskPixelsWithoutG0() const; diff --git a/common/JFJochMessages.h b/common/JFJochMessages.h index 5b64a535..3943028f 100644 --- a/common/JFJochMessages.h +++ b/common/JFJochMessages.h @@ -23,7 +23,7 @@ #include "XrayFluorescenceSpectrum.h" #include "../gemmi_gph/gemmi/symmetry.hpp" -constexpr const uint64_t user_data_release = 6; +constexpr const uint64_t user_data_release = 7; constexpr const uint64_t user_data_magic_number = 0x52320000UL | user_data_release; enum class CBORImageType {START, END, IMAGE, CALIBRATION, METADATA, NONE}; @@ -78,6 +78,23 @@ struct LatticeMessage { gemmi::CrystalSystem crystal_system; }; +// One axis of the sample's transformation chain, in mounting order: base first, sample last. +// +// A CBOR map has no ordering a consumer may rely on - RFC 8949 requires deterministic encoders to +// SORT map keys - so the chain cannot be expressed as the `goniometer` map, which is a DECTRIS +// stream2 field and keyed by axis name. This is sent instead, as an ordered array, and the +// `goniometer` map is still emitted beside it for consumers that only know stream2. +// +// Rotations carry their own angles here. Translations name the chain position only; their per-image +// positions come from the grid scan, which knows the image count and this does not. +struct TransformationAxis { + std::string name; + bool rotation = true; // false = translation, driven by the grid scan + Coord vector = {0, 0, 1}; + double start = 0; // deg for a rotation; unused for a translation + double increment = 0; // deg per image; 0 means the axis does not move +}; + struct SmargonPosition { float phi_deg = 0; float chi_deg = 0; @@ -270,6 +287,9 @@ struct StartMessage { std::vector rois; std::optional grid_scan; + // The sample chain, base -> sample. Authoritative when non-empty; the goniometer map and the + // grid scan remain for consumers that predate it. + std::vector transformations; std::optional goniometer; float detector_translation[3]; diff --git a/docs/CBOR.md b/docs/CBOR.md index de9a6e18..8a4ed9cd 100644 --- a/docs/CBOR.md +++ b/docs/CBOR.md @@ -1,5 +1,11 @@ # CBOR messages +> **Metadata version 7** adds the ordered `transformations` chain and allows a goniometer axis and a +> grid scan to be sent together. A broker and a writer of different releases must not be mixed across +> this change: an older writer ignores `transformations` and reads only the unordered `goniometer` +> map, so a chain whose order matters - anything with a Smargon chi/phi, or a grid scan combined with +> a rotation - is not reproduced. `magic_number` carries the version. + To communicate between FPGA-equipped receiver system and writers, Jungfraujoch is using binary CBOR encoding with tinycbor library (Intel). The protocol is based on and compatible with [DECTRIS Stream2](https://github.com/dectris/documentation/tree/main/stream_v2). @@ -54,6 +60,7 @@ There are minor differences at the moment: | - - helical_step | Array(float) | Translation for helical scan for 1 image \[m\] | | | - - screening_wedge | Array(float) | Wedge for screening \[deg\] (increment would correspond to difference between screening points) | | | grid_scan | object | Grid scan definition (optional). May be sent together with `goniometer`: a grid is often collected at a given head position, recorded as a stationary axis (step 0) | | +| transformations | Array(object) | The sample transformation chain in mounting order, base first: each element has `name`, `type` (`rotation`/`translation`), `vector`, `start` \[deg\], `increment` \[deg/image\]. An ARRAY because the order matters and a CBOR map has none - RFC 8949 has deterministic encoders sort map keys. Authoritative when present; `goniometer` and `grid_scan` describe the same setup for consumers that predate it | | | - n_fast | uint64 | Number of elements along fast axis | | | - n_slow | uint64 | Number of elements along slow axis | | | - step_x_axis | float | Step along X axis, can be negative \[m\] | | diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 15f9e98d..083c0a13 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -15,6 +15,10 @@ This is an UNSTABLE release. It includes many experimental features, as well as * rugnux: the detector geometry is also logged in XDS's convention (`ORGX`/`ORGY`, detector axis vectors, rotation axis), so it can be compared directly with an XDS refinement. * HDF5: a data file missing next to a VDS master now reads as the error-pixel marker instead of zero counts, so those frames are masked rather than silently integrated as blank. * The writer refuses a stream whose start message declares a different pixel format than its images carry, instead of writing a master that does not describe its own data. +* The image stream carries the sample transformation chain (`transformations`) in mounting order, so a goniometer axis, the Smargon chi/phi and a grid stage can be described together and unambiguously. +* Smargon chi/phi are written for a still as well, and are read back from HDF5; before, they were dropped unless the run also had a rotation axis or a grid scan, and nothing read them. + +**CBOR metadata version 7** - a broker and a writer from different releases must not be mixed: an older writer ignores `transformations` and sees only the unordered `goniometer` map. * The image stream and HDF5 now record `mirror_y`, whether the assembled image is mirrored in Y relative to the detector's raw readout, and it is read back. * rugnux: the `.poni` file declares pyFAI's `orientation`, so pyFAI no longer assumes its own default and get the azimuth sense backwards; radial integration is unchanged. * A grid scan and a goniometer axis are no longer alternatives - both can be set, and the grid scan is no longer silently dropped when an axis is present. diff --git a/frame_serialize/CBORStream2Deserializer.cpp b/frame_serialize/CBORStream2Deserializer.cpp index cd7a7249..eec3abd0 100644 --- a/frame_serialize/CBORStream2Deserializer.cpp +++ b/frame_serialize/CBORStream2Deserializer.cpp @@ -1015,6 +1015,43 @@ namespace { } } + // The ordered sample chain (see TransformationAxis). An array, so the mounting order survives - + // a CBOR map's order carries no guarantee. Sent alongside the goniometer map, which says the same + // thing without the order for consumers that only know DECTRIS stream2. + void ProcessTransformations(StartMessage &message, CborValue &value) { + CborValue array_value; + cborErr(cbor_value_enter_container(&value, &array_value)); + + while (!cbor_value_at_end(&array_value)) { + CborValue map_value; + cborErr(cbor_value_enter_container(&array_value, &map_value)); + + TransformationAxis axis; + while (!cbor_value_at_end(&map_value)) { + const auto key = GetCBORString(map_value); + if (key == "name") + axis.name = GetCBORString(map_value); + else if (key == "type") + axis.rotation = (GetCBORString(map_value) == "rotation"); + else if (key == "vector") + axis.vector = GetCoord(map_value); + else if (key == "start") + axis.start = GetCBORFloat(map_value); + else if (key == "increment") + axis.increment = GetCBORFloat(map_value); + else + cbor_value_advance(&map_value); + } + cborErr(cbor_value_leave_container(&array_value, &map_value)); + + if (axis.name.empty()) + throw JFJochException(JFJochExceptionCategory::CBORError, + "Transformation axis without a name"); + message.transformations.push_back(axis); + } + cborErr(cbor_value_leave_container(&value, &array_value)); + } + void ProcessGoniometerMap(StartMessage &message, CborValue &value) { CborValue map_value; @@ -1261,6 +1298,8 @@ namespace { ProcessAxis(value, message.detector_translation); else if (key == "goniometer") ProcessGoniometerMap(message, value); + else if (key == "transformations") + ProcessTransformations(message, value); else if (key == "grid_scan") message.grid_scan = ProcessGridScan(value); else if (key == "pixel_mask_enabled") diff --git a/frame_serialize/CBORStream2Serializer.cpp b/frame_serialize/CBORStream2Serializer.cpp index 936c199c..16c01e85 100644 --- a/frame_serialize/CBORStream2Serializer.cpp +++ b/frame_serialize/CBORStream2Serializer.cpp @@ -358,6 +358,29 @@ inline void CBOR_ENC_GRID_SCAN(CborEncoder &encoder, const char* key, const Grid cborErr(cbor_encoder_close_container(&encoder, &mapEncoder)); } +inline void CBOR_ENC_TRANSFORMATIONS(CborEncoder &encoder, const char* key, + const std::vector &chain) { + if (chain.empty()) + return; + + CborEncoder arrayEncoder, mapEncoder; + cborErr(cbor_encode_text_stringz(&encoder, key)); + // An ARRAY, not a map: the order is the mounting order and must survive. A CBOR map cannot carry + // that - RFC 8949 requires deterministic encoders to sort map keys - which is why this exists + // beside the (unordered, DECTRIS-defined) goniometer map rather than replacing it. + cborErr(cbor_encoder_create_array(&encoder, &arrayEncoder, chain.size())); + for (const auto &axis: chain) { + cborErr(cbor_encoder_create_map(&arrayEncoder, &mapEncoder, 5)); + CBOR_ENC(mapEncoder, "name", axis.name); + CBOR_ENC(mapEncoder, "type", axis.rotation ? "rotation" : "translation"); + CBOR_ENC(mapEncoder, "vector", axis.vector); + CBOR_ENC(mapEncoder, "start", static_cast(axis.start)); + CBOR_ENC(mapEncoder, "increment", static_cast(axis.increment)); + cborErr(cbor_encoder_close_container(&arrayEncoder, &mapEncoder)); + } + cborErr(cbor_encoder_close_container(&encoder, &arrayEncoder)); +} + inline void CBOR_ENC_GONIOMETER_MAP(CborEncoder &encoder, const char* key, const StartMessage &msg) { CborEncoder mapEncoder; @@ -688,7 +711,9 @@ void CBORStream2Serializer::SerializeSequenceStart(const StartMessage& message) CBOR_ENC(mapEncoder, "series_id", message.run_number); CBOR_ENC(mapEncoder, "fluorescence", message.fluorescence_spectrum); - // Both, when both are present - the decoder has always read the two keys independently. + // The ordered chain, and beside it the DECTRIS-shaped goniometer map and the grid scan, which a + // consumer that predates the chain still understands. Both describe the same setup. + CBOR_ENC_TRANSFORMATIONS(mapEncoder, "transformations", message.transformations); if (message.goniometer) CBOR_ENC_GONIOMETER_MAP(mapEncoder, "goniometer", message); if (message.grid_scan) diff --git a/reader/HDF5MetadataSource.cpp b/reader/HDF5MetadataSource.cpp index 65a2cc27..5d79bbb8 100644 --- a/reader/HDF5MetadataSource.cpp +++ b/reader/HDF5MetadataSource.cpp @@ -624,6 +624,24 @@ HDF5MetadataSource::OpenResult HDF5MetadataSource::Open(const std::string &filen } if (stationary.has_value()) dataset->experiment.Goniometer(stationary); + + // chi and phi are ordinary stationary axes in the file; the settings still keep them in + // their own Smargon field, so put them back there. Without this a re-opened file lost + // the head position entirely - nothing in reader/ read it. + const auto chi = ReadAxis(master_file.get(), "chi"); + const auto phi = ReadAxis(master_file.get(), "phi"); + if (chi.has_value() || phi.has_value()) { + SmargonPosition smargon; + if (chi.has_value()) { + smargon.chi_deg = chi->GetStart_deg(); + smargon.chi_axis = chi->GetAxis(); + } + if (phi.has_value()) { + smargon.phi_deg = phi->GetStart_deg(); + smargon.phi_axis = phi->GetAxis(); + } + dataset->experiment.Smargon(smargon); + } } // Independent of the axis: a grid scan can be taken at a given head position, so the two are diff --git a/tests/CBORTest.cpp b/tests/CBORTest.cpp index 08be6b06..d78a158e 100644 --- a/tests/CBORTest.cpp +++ b/tests/CBORTest.cpp @@ -1317,3 +1317,37 @@ TEST_CASE("CBORSerialize_Image_LatticeType", "[CBOR][Lattice]") { i++; } } + +// The chain is sent as an ARRAY because the mounting order has to survive, and a CBOR map's order +// carries no guarantee - RFC 8949 requires deterministic encoders to sort map keys. This asserts the +// order round trips, not merely the contents. +TEST_CASE("CBORSerialize_Start_Transformations", "[CBOR]") { + StartMessage message{}; + message.transformations = { + {.name = "grid_scan_x", .rotation = false, .vector = {1, 0, 0}}, + {.name = "grid_scan_y", .rotation = false, .vector = {0, 1, 0}}, + {.name = "omega", .rotation = true, .vector = {-1, 0, 0}, .start = 95.0, .increment = 0.1}, + {.name = "chi", .rotation = true, .vector = {0, 0, 1}, .start = 12.5}, + {.name = "phi", .rotation = true, .vector = {1, 0, 0}, .start = -7.25}, + }; + + std::vector buffer(MESSAGE_SIZE_FOR_START_END); + CBORStream2Serializer serializer(buffer.data(), buffer.size()); + REQUIRE_NOTHROW(serializer.SerializeSequenceStart(message)); + buffer.resize(serializer.GetBufferSize()); + + auto output = CBORStream2Deserialize(buffer.data(), buffer.size()); + REQUIRE(output->start_message.has_value()); + const auto &chain = output->start_message->transformations; + REQUIRE(chain.size() == message.transformations.size()); + + for (size_t i = 0; i < chain.size(); i++) { + CHECK(chain[i].name == message.transformations[i].name); + CHECK(chain[i].rotation == message.transformations[i].rotation); + CHECK(chain[i].vector.x == Catch::Approx(message.transformations[i].vector.x)); + CHECK(chain[i].vector.y == Catch::Approx(message.transformations[i].vector.y)); + CHECK(chain[i].vector.z == Catch::Approx(message.transformations[i].vector.z)); + CHECK(chain[i].start == Catch::Approx(message.transformations[i].start).margin(1e-4)); + CHECK(chain[i].increment == Catch::Approx(message.transformations[i].increment).margin(1e-6)); + } +}