Tell a Smargon head position from the spindle by equipment_component

The reader recognised chi and phi by name. phi is an ordinary spindle name in MX, so a file whose
rotation axis is called phi had it read back as a head position as well as the spindle - and writing
that experiment out again threw, because the sample chain then tried to create phi twice. In the
other direction a still with a head position had chi, its alphabetically first stationary axis,
adopted as the goniometer.

Both are now settled by the file: the axes jfjoch writes for a Smargon carry
equipment_component="smargon", the reader takes a head position only from a tagged axis, and skips
tagged axes when looking for the spindle. NXmx defines equipment_component as an identifier of the
component of the equipment a transformation belongs to, which is what this is; there is no
"equipment" attribute in NeXus at all.

Adds HDF5Object::AttrExists, since the tag is absent on every file from anywhere else.

The two tests assert on the written file - the axis length and the attribute - because the reader
cannot see either: it does not look at a shape, and it did not look at the tag. That is the same gap
that let the one-image shape through.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z

common/DiffractionExperiment.cpp

@@ -1504,8 +1504,10 @@ std::vector<DetectorTransformation> DiffractionExperiment::BuildTransformationCh
     if (const auto smargon = dataset.GetSmargonPosition()) {
         const auto n = static_cast<size_t>(std::max<int64_t>(image_num, 1));
         add(DetectorTransformation("chi", TransformationType::Rotation, smargon->chi_axis)
+                    .EquipmentComponent("smargon")
                     .Values(std::vector<float>(n, smargon->chi_deg)));
         add(DetectorTransformation("phi", TransformationType::Rotation, smargon->phi_axis)
+                    .EquipmentComponent("smargon")
                     .Values(std::vector<float>(n, smargon->phi_deg)));
     }
 

reader/HDF5MetadataSource.cpp

@@ -609,8 +609,23 @@ HDF5MetadataSource::OpenResult HDF5MetadataSource::Open(const std::string &filen
         // recorded as "phi" back as stills, silently. Prefer an axis that actually turns; fall back
         // to a stationary one, which still says where the head was.
         if (master_file->Exists("/entry/sample/transformations")) {
+            // A Smargon chi/phi is tagged with equipment_component - it is a head position, not the
+            // spindle. Recognised by that tag and not by name: phi is an ordinary spindle name in MX,
+            // so a file from anywhere else must not have its rotation axis read back as a head
+            // position, nor its spindle mistaken for one here.
+            auto is_smargon_axis = [this](const std::string &name) {
+                const std::string dname = "/entry/sample/transformations/" + name;
+                if (!master_file->Exists(dname))
+                    return false;
+                HDF5DataSet axis(*master_file, dname);
+                return axis.AttrExists("equipment_component")
+                       && (axis.ReadAttrStr("equipment_component") == "smargon");
+            };
+
             std::optional<GoniometerAxis> stationary;
             for (const auto &name: master_file->FindLeafs("/entry/sample/transformations")) {
+                if (is_smargon_axis(name))
+                    continue;
                 auto axis = ReadAxis(master_file.get(), name);
                 if (!axis.has_value())
                     continue;
@@ -628,8 +643,11 @@ HDF5MetadataSource::OpenResult HDF5MetadataSource::Open(const std::string &filen
             // 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");
+            std::optional<GoniometerAxis> chi, phi;
+            if (is_smargon_axis("chi"))
+                chi = ReadAxis(master_file.get(), "chi");
+            if (is_smargon_axis("phi"))
+                phi = ReadAxis(master_file.get(), "phi");
             if (chi.has_value() || phi.has_value()) {
                 SmargonPosition smargon;
                 if (chi.has_value()) {

tests/JFJochReaderTest.cpp

@@ -2995,3 +2995,130 @@ TEST_CASE("JFJochReader_TransformationChain_SentAndBuilt", "[HDF5][Full]") {
     remove("test_chain_sent_master.h5");
     REQUIRE(H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL) == 0);
 }
+
+// A head position is not a sweep, and it is not the spindle either. Both properties are carried by
+// the file itself - the axis length says how many images there are, the equipment_component tag says
+// what the axis is - so both are checked here on the file, not through the reader: the reader alone
+// cannot see a shape or an attribute it never looks at.
+TEST_CASE("JFJochReader_Smargon_StillIsNotOneImage", "[HDF5][Full]") {
+    DiffractionExperiment x(DetJF(1));
+    x.ImagesPerTrigger(5).OverwriteExistingFiles(true).FilePrefix("test_smargon");
+    x.BeamX_pxl(100).BeamY_pxl(200).DetectorDistance_mm(150)
+            .IncidentEnergy_keV(WVL_1A_IN_KEV).PixelSigned(false).BitDepthImage(16)
+            .FrameTime(std::chrono::microseconds(500), std::chrono::microseconds(10));
+    x.Smargon(SmargonPosition{.phi_deg = -7.25f, .chi_deg = 12.5f});
+    REQUIRE(!x.GetGoniometer().has_value());
+
+    RegisterHDF5Filter();
+    std::vector<uint16_t> image(x.GetPixelsNum(), 0);
+
+    StartMessage start_message;
+    x.FillMessage(start_message);
+    FileWriter file_set(start_message);
+    DataMessage message{};
+    for (int i = 0; i < x.GetImageNum(); i++) {
+        message.image = CompressedImage(image, x.GetXPixelsNum(), x.GetYPixelsNum());
+        message.number = i;
+        REQUIRE_NOTHROW(file_set.WriteHDF5(message));
+    }
+    EndMessage end_message;
+    end_message.max_image_number = x.GetImageNum();
+    file_set.WriteHDF5(end_message);
+    file_set.Finalize();
+
+    {
+        HDF5ReadOnlyFile master("test_smargon_master.h5");
+
+        // One entry per image. A reader takes the image count from the innermost axis of the sample
+        // chain when no axis varies; as scalars these read back as a single image.
+        CHECK(master.GetDimension("/entry/sample/transformations/chi")
+              == std::vector<hsize_t>{static_cast<hsize_t>(x.GetImageNum())});
+        CHECK(master.GetDimension("/entry/sample/transformations/phi")
+              == std::vector<hsize_t>{static_cast<hsize_t>(x.GetImageNum())});
+        CHECK(master.ReadVector<double>("/entry/sample/transformations/phi")
+              == std::vector<double>(x.GetImageNum(), -7.25));
+
+        // Tagged, so neither is mistaken for the spindle - and so a phi from anywhere else is not
+        // mistaken for a head position.
+        HDF5DataSet chi(master, "/entry/sample/transformations/chi");
+        HDF5DataSet phi(master, "/entry/sample/transformations/phi");
+        REQUIRE(chi.AttrExists("equipment_component"));
+        REQUIRE(phi.AttrExists("equipment_component"));
+        CHECK(chi.ReadAttrStr("equipment_component") == "smargon");
+        CHECK(phi.ReadAttrStr("equipment_component") == "smargon");
+    }
+
+    const auto read = [](const std::string &prefix) {
+        JFJochHDF5Reader reader;
+        reader.ReadFile(prefix + "_master.h5");
+        return reader.GetDataset()->experiment;
+    };
+    const auto read_back = read("test_smargon");
+
+    // chi is the alphabetically first stationary axis in the file; it must not become the spindle.
+    CHECK(!read_back.GetGoniometer().has_value());
+    REQUIRE(read_back.GetDatasetSettings().GetSmargonPosition().has_value());
+    CHECK(read_back.GetDatasetSettings().GetSmargonPosition()->chi_deg == Catch::Approx(12.5f).margin(1e-3));
+    CHECK(read_back.GetDatasetSettings().GetSmargonPosition()->phi_deg == Catch::Approx(-7.25f).margin(1e-3));
+
+    remove("test_smargon_master.h5");
+    remove("test_smargon_data_000001.h5");
+    REQUIRE(H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL) == 0);
+}
+
+// phi is an ordinary spindle name in MX. A file whose rotation axis is called phi carries no
+// equipment_component, so it stays the spindle and no head position is invented from it - which also
+// means the file can be written back out, instead of colliding on a second dataset called phi.
+TEST_CASE("JFJochReader_Goniometer_NamedPhiIsNotSmargon", "[HDF5][Full]") {
+    DiffractionExperiment x(DetJF(1));
+    x.ImagesPerTrigger(5).OverwriteExistingFiles(true).FilePrefix("test_phispindle");
+    x.BeamX_pxl(100).BeamY_pxl(200).DetectorDistance_mm(150)
+            .IncidentEnergy_keV(WVL_1A_IN_KEV).PixelSigned(false).BitDepthImage(16)
+            .FrameTime(std::chrono::microseconds(500), std::chrono::microseconds(10));
+    x.Goniometer(GoniometerAxis("phi", 30, 0.2f, Coord(0,-1,0), {}));
+
+    RegisterHDF5Filter();
+    std::vector<uint16_t> image(x.GetPixelsNum(), 0);
+
+    StartMessage start_message;
+    x.FillMessage(start_message);
+    FileWriter file_set(start_message);
+    DataMessage message{};
+    for (int i = 0; i < x.GetImageNum(); i++) {
+        message.image = CompressedImage(image, x.GetXPixelsNum(), x.GetYPixelsNum());
+        message.number = i;
+        REQUIRE_NOTHROW(file_set.WriteHDF5(message));
+    }
+    EndMessage end_message;
+    end_message.max_image_number = x.GetImageNum();
+    file_set.WriteHDF5(end_message);
+    file_set.Finalize();
+
+    const auto read = [](const std::string &prefix) {
+        JFJochHDF5Reader reader;
+        reader.ReadFile(prefix + "_master.h5");
+        return reader.GetDataset()->experiment;
+    };
+    const auto read_back = read("test_phispindle");
+
+    REQUIRE(read_back.GetGoniometer().has_value());
+    CHECK(read_back.GetGoniometer()->GetName() == "phi");
+    CHECK(read_back.GetGoniometer()->GetStart_deg() == Catch::Approx(30).margin(1e-3));
+    CHECK(!read_back.GetDatasetSettings().GetSmargonPosition().has_value());
+
+    // Writing what was read must not try to create phi a second time.
+    DiffractionExperiment rewrite = read_back;
+    rewrite.FilePrefix("test_phispindle_out").OverwriteExistingFiles(true);
+    StartMessage out_start;
+    rewrite.FillMessage(out_start);
+    FileWriter out(out_start);
+    EndMessage out_end;
+    out_end.max_image_number = rewrite.GetImageNum();
+    REQUIRE_NOTHROW(out.WriteHDF5(out_end));
+    REQUIRE_NOTHROW(out.Finalize());
+
+    remove("test_phispindle_master.h5");
+    remove("test_phispindle_data_000001.h5");
+    remove("test_phispindle_out_master.h5");
+    REQUIRE(H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL) == 0);
+}

writer/HDF5NXmx.cpp

@@ -769,12 +769,12 @@ void NXmx::Sample(const StartMessage &start, const EndMessage &end) {
         const auto n = static_cast<size_t>(std::max<int64_t>(end.max_image_number, 1));
 
         SaveVector(transformations, "chi", std::vector<double>(n, start.smargon_position->chi_deg))->
-                Transformation("deg", depends_on, "", "", "rotation",
+                Transformation("deg", depends_on, "", "smargon", "rotation",
                     {start.smargon_position->chi_axis.x, start.smargon_position->chi_axis.y,
                      start.smargon_position->chi_axis.z}, {0, 0, 0}, "");
         depends_on = "/entry/sample/transformations/chi";
         SaveVector(transformations, "phi", std::vector<double>(n, start.smargon_position->phi_deg))->
-                Transformation("deg", depends_on, "", "", "rotation",
+                Transformation("deg", depends_on, "", "smargon", "rotation",
                     {start.smargon_position->phi_axis.x, start.smargon_position->phi_axis.y,
                      start.smargon_position->phi_axis.z}, {0, 0, 0}, "");
         depends_on = "/entry/sample/transformations/phi";

writer/HDF5Objects.cpp

@@ -534,6 +534,10 @@ std::vector<double> HDF5Object::ReadAttrVec(const std::string &name) {
     return ret;
 }
 
+bool HDF5Object::AttrExists(const std::string &name) {
+    return H5Aexists(id, name.c_str()) > 0;
+}
+
 std::string HDF5Object::ReadAttrStr(const std::string &name) {
     hid_t attr_id = H5Aopen(id, name.c_str(), H5P_DEFAULT);
     if (attr_id < 0)

writer/HDF5Objects.h

@@ -138,6 +138,7 @@ public:
     HDF5Object& Attr(const std::string& name, const std::vector<double> &val);
 
     std::vector<double> ReadAttrVec(const std::string& name);
+    bool AttrExists(const std::string &name);
     std::string ReadAttrStr(const std::string &name);
     double ReadAttrDouble(const std::string &name);
     int64_t ReadAttrInt(const std::string &name);