leonarski_fandClaude Opus 5
b2c0a5ea48
From two independent code reviews of the model-validation work. Nothing here changes a verdict: the acceptance set still reads +17.69 / +1.70 / -0.37 sigma and the rejected runs still write files byte-identical to a run with no model. THE RUN'S OUTPUT. Model validation runs BEFORE the reflection files are written, and two paths through it could throw: the null's replicates (rotated models fed to a scaling path that fails outright on data it cannot pair up - the adversarial input for it), and the mmCIF coordinate writer. Either would have taken the .mtz, .cif, .hkl and _unmerged.mtz with it, after the merge had already been paid for. A null that cannot be built is a question that could not be put, which is the NOT_TESTED state this design already has; a coordinate file that cannot be written is a lost convenience. Both now degrade instead of aborting. THE ORIGIN GAUGE. Translating the whole cell content along a free-origin direction - all three in P1, the unique axis in a polar group - leaves every |F| exactly unchanged. The code said the LM damping and the R-free gate made that harmless between them. Neither does: the gate is a function of |F| and is blind to exactly this, and the gauge column of the Jacobian is not zero but noise divided by the difference step. It is now projected out after every zone, against the group's own common fixed subspace. P2_1 alone is a large share of deposited structures, and the reported shift was partly fiction in every one of them. TWO CLAIMS THAT WERE FALSE. The report told the user R-work carries the decision "because nothing was refined against it", six lines from where six placement parameters are refined against it; the real argument is that the null is placed the same way, so the optimism is common-mode and cancels. And the constant's own comment quoted a +4 vs +1 sigma gap where the measurement is +17.7 vs +1.7. Also: the map file's phase columns are back to [0, 360), the convention they carried before sigma_A weighting; with no data space group the map file follows the reflections into P1 rather than taking the model's group; the sigma is floored against a near-zero null spread rather than only an exactly-zero one; the model is restored whether or not the solver reported a usable answer; and the zone list is the ladder walked rather than the ladder planned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
50 lines
2.3 KiB
C++
50 lines
2.3 KiB
C++
// SPDX-FileCopyrightText: 2026 Filip Leonarski, Paul Scherrer Institute <filip.leonarski@psi.ch>
|
|
// SPDX-License-Identifier: GPL-3.0-only
|
|
|
|
#include "WriteModel.h"
|
|
|
|
#include <fstream>
|
|
|
|
#include <gemmi/to_mmcif.hpp> // make_mmcif_document
|
|
#include <gemmi/to_cif.hpp> // write_cif_to_stream
|
|
#include <gemmi/polyheur.hpp> // setup_entities
|
|
|
|
#include "../common/Logger.h"
|
|
|
|
void WritePlacedModel(const gemmi::Structure &placed,
|
|
const UnitCell &cell,
|
|
const gemmi::SpaceGroup &space_group,
|
|
const std::string &output_prefix,
|
|
Logger &logger) {
|
|
const std::string path = output_prefix + "_model.cif";
|
|
|
|
gemmi::Structure st = placed;
|
|
// Not the model's own cell and group but the data's, taken from the same two values the reflection
|
|
// files are written from. The coordinates already sit in this cell; what is set here is the label
|
|
// the file carries, which must match the .mtz beside it - the enantiomorph in particular, since
|
|
// --model can adopt the model's and that is neither the data's original label nor, necessarily,
|
|
// the one the input model arrived with.
|
|
st.cell = cell;
|
|
st.spacegroup_hm = space_group.xhm();
|
|
st.setup_cell_images();
|
|
// Fills in entity types and label_asym_id for a model read from a PDB, which carries neither.
|
|
// Both are no-ops where the input already had them, i.e. for an mmCIF input.
|
|
// setup_entities and make_mmcif_document can both throw, and this runs BEFORE the reflection files
|
|
// are written: a convenience deliverable must not be able to take the run's actual output with it.
|
|
// The map writer does the same.
|
|
try {
|
|
gemmi::setup_entities(st);
|
|
std::ofstream os(path);
|
|
gemmi::cif::write_cif_to_stream(os, gemmi::make_mmcif_document(st));
|
|
if (!os) {
|
|
logger.Error("Model validation: cannot write the placed model to {}", path);
|
|
return;
|
|
}
|
|
} catch (const std::exception &e) {
|
|
logger.Warning("Model validation: could not write the placed model to {} ({})", path, e.what());
|
|
return;
|
|
}
|
|
logger.Info("Model validation: the model as placed against these data written to {} "
|
|
"(cell and space group {} as in the reflection files)", path, space_group.short_name());
|
|
}
|