model: the placed coordinates carry the group they obey, not the data's

WritePlacedModel stamped the reflection files' space group on the placed
model unconditionally. That is right whenever the coordinates obey that
group - the ordinary case, and the subgroup case too, where the file just
holds more than one asymmetric unit of it - but wrong when the data were
merged in a supergroup of the model's group, or in the other hand. The
extra operations are then a claim the contents contradict: a refinement
program expands them, refines a structure that is not the model, and
reports nothing.

Measured on a public deposition whose data this run over-merged across a
pseudo-symmetry twofold: refining the written pair gave R-free 0.495,
where the same coordinates and the same reflections under the model's own
group give 0.285. The label alone was worth 0.21 of R-free.

The group is now taken from the argument only where every one of its
operations is one the model already has, and the model keeps its own
otherwise. The two files then disagree, which is the truth about them and
is warned about, rather than agreeing on something false.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-09-16 11:52:24 +02:00
co-authored by Claude Opus 5
parent 0080d2f4ba
commit 7dc066c0f3
3 changed files with 112 additions and 13 deletions
+62 -2
View File
@@ -398,9 +398,12 @@ TEST_CASE("WriteModel_KeepsTheContentAndTakesTheGivenFrame", "[ModelValidation]"
const auto input = WriteTemp("write_model_test_input.pdb", kPdbRich);
gemmi::Structure st = gemmi::read_structure_gz(input, gemmi::CoorFormat::Detect);
// The model in the data's own group, which is the ordinary case: the frame it is written in is
// then wholly the caller's. The cell it arrives in (40/50/60) is not the data's and is replaced.
st.spacegroup_hm = "P 43 21 2";
// A frame that is neither the model's (P 1, 40/50/60) nor anything derived from it: the tetragonal
// lysozyme cell and one enantiomorph of its group, standing in for what AdoptModelFrame settled.
// The tetragonal lysozyme cell and one enantiomorph of its group, standing in for what
// AdoptModelFrame settled.
const UnitCell data_cell{.a = 79, .b = 79, .c = 38, .alpha = 90, .beta = 90, .gamma = 90};
const gemmi::SpaceGroup *sg = gemmi::find_spacegroup_by_name("P 43 21 2");
REQUIRE(sg != nullptr);
@@ -441,6 +444,63 @@ TEST_CASE("WriteModel_KeepsTheContentAndTakesTheGivenFrame", "[ModelValidation]"
std::filesystem::remove(written);
}
// A model cannot be labelled with a group its own coordinates do not obey. Where the data were merged
// in a supergroup of the model's - an over-merge across a pseudo-symmetry operation, or the hand the
// model's fit did not earn - that operation would generate atoms the model does not contain, and a
// refinement program acts on it silently. The model keeps its own group and the caller is warned.
TEST_CASE("WriteModel_DoesNotStampAGroupTheCoordinatesDoNotObey", "[ModelValidation]") {
Logger logger("WriteModel_DoesNotStampAGroupTheCoordinatesDoNotObey");
const auto input = WriteTemp("write_model_sg_test_input.pdb", kPdbRich); // P 1
const gemmi::Structure st = gemmi::read_structure_gz(input, gemmi::CoorFormat::Detect);
const UnitCell data_cell{.a = 79, .b = 79, .c = 38, .alpha = 90, .beta = 90, .gamma = 90};
const gemmi::SpaceGroup *sg = gemmi::find_spacegroup_by_name("P 43 21 2");
REQUIRE(sg != nullptr);
WritePlacedModel(st, data_cell, *sg, "write_model_sg_test", logger);
const std::string written = "write_model_sg_test_model.cif";
REQUIRE(std::filesystem::exists(written));
const gemmi::Structure back = gemmi::read_structure_gz(written, gemmi::CoorFormat::Detect);
// The cell is still the data's - the coordinates do sit in it - but the group is the model's own.
CHECK(back.cell.a == Catch::Approx(79.0));
CHECK(back.cell.c == Catch::Approx(38.0));
REQUIRE(back.find_spacegroup() != nullptr);
CHECK(back.find_spacegroup()->number == 1);
std::filesystem::remove(input);
std::filesystem::remove(written);
std::filesystem::remove("write_model_sg_test_model.pdb");
}
// The other direction is not a mismatch at all: a model whose coordinates obey more symmetry than the
// data were merged in obeys the data's group too - the file simply holds more than one asymmetric unit
// of it, which is what a model in P1 always does. The reflection files' group is kept, as before.
TEST_CASE("WriteModel_KeepsTheDataGroupWhenItIsASubgroup", "[ModelValidation]") {
Logger logger("WriteModel_KeepsTheDataGroupWhenItIsASubgroup");
const auto input = WriteTemp("write_model_sub_test_input.pdb", kPdbRich);
gemmi::Structure st = gemmi::read_structure_gz(input, gemmi::CoorFormat::Detect);
st.spacegroup_hm = "P 43 21 2";
const UnitCell data_cell{.a = 79, .b = 79, .c = 38, .alpha = 90, .beta = 90, .gamma = 90};
const gemmi::SpaceGroup *p1 = gemmi::find_spacegroup_by_name("P 1");
REQUIRE(p1 != nullptr);
WritePlacedModel(st, data_cell, *p1, "write_model_sub_test", logger);
const std::string written = "write_model_sub_test_model.cif";
REQUIRE(std::filesystem::exists(written));
const gemmi::Structure back = gemmi::read_structure_gz(written, gemmi::CoorFormat::Detect);
REQUIRE(back.find_spacegroup() != nullptr);
CHECK(back.find_spacegroup()->number == 1);
std::filesystem::remove(input);
std::filesystem::remove(written);
std::filesystem::remove("write_model_sub_test_model.pdb");
}
// CC(model, data) has to follow where the signal actually is, or it cannot support the one-sided
// claim it exists for. The check is closed: the "observed" intensities are the model's own with
// Gaussian noise added, and how much noise is chosen per shell - almost none in the first, some in