rugnux: code-review fixes across the offline path
- frontend: drop the CBF/TIFF file-format menu items; the broker now rejects those formats, so offering them in the picker was a dead, error-producing choice (the enum values remain only for back-compat decode). - RugnuxCommandLine: emit --background-trim (when it differs from the 0.10 default) and --stills-modulation, so the viewer "copy command line" matches what the settings dock actually configured. - ReindexAmbiguity: multiply the reference-CC intensity by rlp rather than dividing by it, matching image_scale_corr everywhere else; drop the now unused SafeInv helper. - rugnux_cli: parse --refine-geometry with the hardened bounded parser instead of atoi (reject trailing garbage; bound N so refine_frames*50 can't overflow). - LoadFCalcFromMtz: drop the generic "first type-F column" catch-all so a reference MTZ carrying only map coefficients (FWT/FC_ALL) can't silently seed CCref from the wrong quantity. - WriteReflections: consolidate the duplicated MTZ Friedel/anomalous branches into one shared row list + column/row emission (behaviour-preserving). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 4.8
@@ -75,8 +75,6 @@ function FileWriterSettings({s: serverS}: MyProps) {
|
||||
<MenuItem value={file_writer_format.N_XMX_VDS}>NXmx HDF5 master file with virtual datasets</MenuItem>
|
||||
<MenuItem value={file_writer_format.N_XMX_INTEGRATED}>Single HDF5 file with data and metadata</MenuItem>
|
||||
<MenuItem value={file_writer_format.N_XMX_ONLY_DATA}>No NXmx HDF5 master file (only data files)</MenuItem>
|
||||
<MenuItem value={file_writer_format.CBF}>miniCBF (only data files; limited metadata)</MenuItem>
|
||||
<MenuItem value={file_writer_format.TIFF}>TIFF (only data files; no metadata)</MenuItem>
|
||||
<MenuItem value={file_writer_format.NO_FILE_WRITTEN}>No files saved</MenuItem>
|
||||
</Select>
|
||||
</FormControl>
|
||||
|
||||
@@ -39,18 +39,15 @@ const gemmi::Mtz::Column* SelectDefaultColumn(const gemmi::Mtz& mtz, bool& squar
|
||||
// Fall back to a plain structure-factor amplitude (type F): a deposited/observed FP or FOBS, or a
|
||||
// lone FC, squared to an intensity like F-model. Lets a reference MTZ that carries only amplitudes
|
||||
// (no F-model or IMEAN/I - e.g. a deposition that has only FP) still seed CCref
|
||||
// without an explicit --reference-column.
|
||||
// without an explicit --reference-column. Only these named observed/calculated labels are matched:
|
||||
// a bare "first type-F column" catch-all would silently pick up map coefficients (FWT, DELFWT,
|
||||
// FC_ALL) and seed CCref from the wrong quantity - require --reference-column in that case instead.
|
||||
for (const char* label : {"FP", "FOBS", "F", "FC", "Fobs", "F-obs"}) {
|
||||
if (const auto* col = mtz.column_with_label(label, nullptr, 'F')) {
|
||||
square = true;
|
||||
return col;
|
||||
}
|
||||
}
|
||||
for (const auto& c : mtz.columns)
|
||||
if (c.type == 'F') {
|
||||
square = true;
|
||||
return &c;
|
||||
}
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
|
||||
@@ -286,141 +286,121 @@ void WriteMtzReflections(const std::vector<MergedReflection> &reflections,
|
||||
|
||||
const int dataset_id = ds.id;
|
||||
|
||||
// In anomalous mode the merge keeps the two Friedel mates as separate rows (I+ under the ASU
|
||||
// representative hkl, I- under -hkl). Emitting those verbatim gives a file with two rows per
|
||||
// reflection that downstream tools have to re-collapse. Instead pair the mates into one row per
|
||||
// reflection with the standard CCP4 anomalous layout (IMEAN + I(+)/I(-), and the same split for
|
||||
// the French-Wilson amplitude), which aimless / ctruncate / mtz2sca / ANODE read directly.
|
||||
if (experiment.GetScalingSettings().GetMergeFriedel()) {
|
||||
// Friedel-merged output: IMEAN is the primary intensity. I(+)/I(-) are written by default too
|
||||
// when the merge kept a Bijvoet split (rotation always does; the split is scaled non-anomalously),
|
||||
// so a weak anomalous signal is preserved without reprocessing. A reflection with only one mate,
|
||||
// or a centric, gets a missing value (NaN) for the absent hand. When no reflection has an
|
||||
// anomalous split (e.g. the stills path, which does not compute one) the columns are omitted.
|
||||
const bool has_anom = std::any_of(reflections.begin(), reflections.end(),
|
||||
[](const MergedReflection& r){ return std::isfinite(r.I_plus) || std::isfinite(r.I_minus); });
|
||||
mtz.add_column("H", 'H', dataset_id, -1, false);
|
||||
mtz.add_column("K", 'H', dataset_id, -1, false);
|
||||
mtz.add_column("L", 'H', dataset_id, -1, false);
|
||||
mtz.add_column("IMEAN", 'J', dataset_id, -1, false);
|
||||
mtz.add_column("SIGIMEAN", 'Q', dataset_id, -1, false);
|
||||
if (has_anom) {
|
||||
mtz.add_column("I(+)", 'K', dataset_id, -1, false);
|
||||
mtz.add_column("SIGI(+)", 'M', dataset_id, -1, false);
|
||||
mtz.add_column("I(-)", 'K', dataset_id, -1, false);
|
||||
mtz.add_column("SIGI(-)", 'M', dataset_id, -1, false);
|
||||
}
|
||||
mtz.add_column("F", 'F', dataset_id, -1, false); // French-Wilson amplitude
|
||||
mtz.add_column("SIGF", 'Q', dataset_id, -1, false);
|
||||
if (has_anom) {
|
||||
mtz.add_column("F(+)", 'G', dataset_id, -1, false);
|
||||
mtz.add_column("SIGF(+)", 'L', dataset_id, -1, false);
|
||||
mtz.add_column("F(-)", 'G', dataset_id, -1, false);
|
||||
mtz.add_column("SIGF(-)", 'L', dataset_id, -1, false);
|
||||
}
|
||||
mtz.add_column("FreeR_flag", 'I', dataset_id, -1, false);
|
||||
|
||||
mtz.nreflections = static_cast<int>(reflections.size());
|
||||
mtz.data.reserve(reflections.size() * (has_anom ? 16 : 8));
|
||||
for (const auto& r : reflections) {
|
||||
mtz.data.push_back(static_cast<float>(r.h));
|
||||
mtz.data.push_back(static_cast<float>(r.k));
|
||||
mtz.data.push_back(static_cast<float>(r.l));
|
||||
mtz.data.push_back(r.I);
|
||||
mtz.data.push_back(r.sigma);
|
||||
if (has_anom) {
|
||||
mtz.data.push_back(r.I_plus);
|
||||
mtz.data.push_back(r.sigma_plus);
|
||||
mtz.data.push_back(r.I_minus);
|
||||
mtz.data.push_back(r.sigma_minus);
|
||||
}
|
||||
mtz.data.push_back(r.F);
|
||||
mtz.data.push_back(r.sigmaF);
|
||||
if (has_anom) {
|
||||
mtz.data.push_back(r.F_plus);
|
||||
mtz.data.push_back(r.sigmaF_plus);
|
||||
mtz.data.push_back(r.F_minus);
|
||||
mtz.data.push_back(r.sigmaF_minus);
|
||||
}
|
||||
mtz.data.push_back(r.rfree_flag ? 1.0f : 0.0f);
|
||||
}
|
||||
mtz.write_to_file(filename);
|
||||
return;
|
||||
}
|
||||
|
||||
// Anomalous: group the two mates by their (shared) Friedel-merged ASU representative. A single
|
||||
// generator gives both the group key (its hkl, identical for +hkl and -hkl) and which mate this
|
||||
// row is (.plus).
|
||||
const HKLKeyGenerator key_gen(false, experiment.GetSpaceGroupNumber().value_or(1));
|
||||
|
||||
struct AnomRow {
|
||||
// The merge keeps the two Friedel mates as separate rows (I+ under the ASU representative hkl, I-
|
||||
// under -hkl). Both output modes collapse them into one row per reflection with the standard CCP4
|
||||
// anomalous layout (IMEAN + I(+)/I(-), and the same split for the French-Wilson amplitude), which
|
||||
// aimless / ctruncate / mtz2sca / ANODE read directly. The modes differ only in how each row is
|
||||
// formed, so they build one common row list and share the column/row emission below.
|
||||
struct OutRow {
|
||||
int h = 0, k = 0, l = 0;
|
||||
float Ip = NAN, sIp = NAN, Im = NAN, sIm = NAN;
|
||||
float Fp = NAN, sFp = NAN, Fm = NAN, sFm = NAN;
|
||||
float Imean = NAN, sImean = NAN, Ip = NAN, sIp = NAN, Im = NAN, sIm = NAN;
|
||||
float Fmean = NAN, sFmean = NAN, Fp = NAN, sFp = NAN, Fm = NAN, sFm = NAN;
|
||||
int rfree = 0;
|
||||
};
|
||||
std::map<std::tuple<int, int, int>, AnomRow> rows;
|
||||
for (const auto& r : reflections) {
|
||||
const HKLKey key = key_gen(r);
|
||||
AnomRow& row = rows[{key.h, key.k, key.l}];
|
||||
row.h = key.h; row.k = key.k; row.l = key.l;
|
||||
row.rfree = r.rfree_flag ? 1 : 0;
|
||||
if (key.plus) { row.Ip = r.I; row.sIp = r.sigma; row.Fp = r.F; row.sFp = r.sigmaF; }
|
||||
else { row.Im = r.I; row.sIm = r.sigma; row.Fm = r.F; row.sFm = r.sigmaF; }
|
||||
}
|
||||
std::vector<OutRow> out_rows;
|
||||
bool has_anom = true;
|
||||
|
||||
// Friedel-mean of the two mates by inverse variance (the single mate, if only one was measured).
|
||||
const auto combine = [](float a, float sa, float b, float sb, float& val, float& sig) {
|
||||
const bool ok_a = std::isfinite(a) && sa > 0.0f;
|
||||
const bool ok_b = std::isfinite(b) && sb > 0.0f;
|
||||
if (ok_a && ok_b) {
|
||||
const double wa = 1.0 / (static_cast<double>(sa) * sa);
|
||||
const double wb = 1.0 / (static_cast<double>(sb) * sb);
|
||||
val = static_cast<float>((wa * a + wb * b) / (wa + wb));
|
||||
sig = static_cast<float>(1.0 / std::sqrt(wa + wb));
|
||||
} else if (ok_a) { val = a; sig = sa; }
|
||||
else if (ok_b) { val = b; sig = sb; }
|
||||
else { val = NAN; sig = NAN; }
|
||||
};
|
||||
if (experiment.GetScalingSettings().GetMergeFriedel()) {
|
||||
// Friedel-merged: IMEAN is the already-merged intensity (r.I). I(+)/I(-) are carried verbatim
|
||||
// from the Bijvoet split the merge kept (rotation always does; scaled non-anomalously), so a weak
|
||||
// anomalous signal is preserved without reprocessing. A reflection with only one mate, or a
|
||||
// centric, gets a missing value (NaN) for the absent hand. When NO reflection has an anomalous
|
||||
// split (e.g. the stills path, which does not compute one) the anomalous columns are omitted.
|
||||
has_anom = std::any_of(reflections.begin(), reflections.end(),
|
||||
[](const MergedReflection& r){ return std::isfinite(r.I_plus) || std::isfinite(r.I_minus); });
|
||||
out_rows.reserve(reflections.size());
|
||||
for (const auto& r : reflections)
|
||||
out_rows.push_back({r.h, r.k, r.l, r.I, r.sigma, r.I_plus, r.sigma_plus, r.I_minus,
|
||||
r.sigma_minus, r.F, r.sigmaF, r.F_plus, r.sigmaF_plus, r.F_minus,
|
||||
r.sigmaF_minus, r.rfree_flag ? 1 : 0});
|
||||
} else {
|
||||
// Anomalous: group the two mates by their (shared) Friedel-merged ASU representative, then form
|
||||
// IMEAN / F as their inverse-variance Friedel mean. A single generator gives both the group key
|
||||
// (its hkl, identical for +hkl and -hkl) and which mate this row is (.plus).
|
||||
const HKLKeyGenerator key_gen(false, experiment.GetSpaceGroupNumber().value_or(1));
|
||||
struct AnomRow {
|
||||
int h = 0, k = 0, l = 0;
|
||||
float Ip = NAN, sIp = NAN, Im = NAN, sIm = NAN;
|
||||
float Fp = NAN, sFp = NAN, Fm = NAN, sFm = NAN;
|
||||
int rfree = 0;
|
||||
};
|
||||
std::map<std::tuple<int, int, int>, AnomRow> rows;
|
||||
for (const auto& r : reflections) {
|
||||
const HKLKey key = key_gen(r);
|
||||
AnomRow& row = rows[{key.h, key.k, key.l}];
|
||||
row.h = key.h; row.k = key.k; row.l = key.l;
|
||||
row.rfree = r.rfree_flag ? 1 : 0;
|
||||
if (key.plus) { row.Ip = r.I; row.sIp = r.sigma; row.Fp = r.F; row.sFp = r.sigmaF; }
|
||||
else { row.Im = r.I; row.sIm = r.sigma; row.Fm = r.F; row.sFm = r.sigmaF; }
|
||||
}
|
||||
|
||||
// Friedel-mean of the two mates by inverse variance (the single mate, if only one was measured).
|
||||
const auto combine = [](float a, float sa, float b, float sb, float& val, float& sig) {
|
||||
const bool ok_a = std::isfinite(a) && sa > 0.0f;
|
||||
const bool ok_b = std::isfinite(b) && sb > 0.0f;
|
||||
if (ok_a && ok_b) {
|
||||
const double wa = 1.0 / (static_cast<double>(sa) * sa);
|
||||
const double wb = 1.0 / (static_cast<double>(sb) * sb);
|
||||
val = static_cast<float>((wa * a + wb * b) / (wa + wb));
|
||||
sig = static_cast<float>(1.0 / std::sqrt(wa + wb));
|
||||
} else if (ok_a) { val = a; sig = sa; }
|
||||
else if (ok_b) { val = b; sig = sb; }
|
||||
else { val = NAN; sig = NAN; }
|
||||
};
|
||||
|
||||
out_rows.reserve(rows.size());
|
||||
for (const auto& [hkl, row] : rows) {
|
||||
float i_mean, sig_i_mean, f_mean, sig_f_mean;
|
||||
combine(row.Ip, row.sIp, row.Im, row.sIm, i_mean, sig_i_mean);
|
||||
combine(row.Fp, row.sFp, row.Fm, row.sFm, f_mean, sig_f_mean);
|
||||
out_rows.push_back({row.h, row.k, row.l, i_mean, sig_i_mean, row.Ip, row.sIp, row.Im,
|
||||
row.sIm, f_mean, sig_f_mean, row.Fp, row.sFp, row.Fm, row.sFm, row.rfree});
|
||||
}
|
||||
}
|
||||
|
||||
mtz.add_column("H", 'H', dataset_id, -1, false);
|
||||
mtz.add_column("K", 'H', dataset_id, -1, false);
|
||||
mtz.add_column("L", 'H', dataset_id, -1, false);
|
||||
mtz.add_column("IMEAN", 'J', dataset_id, -1, false);
|
||||
mtz.add_column("SIGIMEAN", 'Q', dataset_id, -1, false);
|
||||
mtz.add_column("I(+)", 'K', dataset_id, -1, false);
|
||||
mtz.add_column("SIGI(+)", 'M', dataset_id, -1, false);
|
||||
mtz.add_column("I(-)", 'K', dataset_id, -1, false);
|
||||
mtz.add_column("SIGI(-)", 'M', dataset_id, -1, false);
|
||||
mtz.add_column("F", 'F', dataset_id, -1, false); // French-Wilson amplitude (mean)
|
||||
if (has_anom) {
|
||||
mtz.add_column("I(+)", 'K', dataset_id, -1, false);
|
||||
mtz.add_column("SIGI(+)", 'M', dataset_id, -1, false);
|
||||
mtz.add_column("I(-)", 'K', dataset_id, -1, false);
|
||||
mtz.add_column("SIGI(-)", 'M', dataset_id, -1, false);
|
||||
}
|
||||
mtz.add_column("F", 'F', dataset_id, -1, false); // French-Wilson amplitude
|
||||
mtz.add_column("SIGF", 'Q', dataset_id, -1, false);
|
||||
mtz.add_column("F(+)", 'G', dataset_id, -1, false);
|
||||
mtz.add_column("SIGF(+)", 'L', dataset_id, -1, false);
|
||||
mtz.add_column("F(-)", 'G', dataset_id, -1, false);
|
||||
mtz.add_column("SIGF(-)", 'L', dataset_id, -1, false);
|
||||
if (has_anom) {
|
||||
mtz.add_column("F(+)", 'G', dataset_id, -1, false);
|
||||
mtz.add_column("SIGF(+)", 'L', dataset_id, -1, false);
|
||||
mtz.add_column("F(-)", 'G', dataset_id, -1, false);
|
||||
mtz.add_column("SIGF(-)", 'L', dataset_id, -1, false);
|
||||
}
|
||||
mtz.add_column("FreeR_flag", 'I', dataset_id, -1, false);
|
||||
|
||||
mtz.nreflections = static_cast<int>(rows.size());
|
||||
mtz.data.reserve(rows.size() * 16);
|
||||
for (const auto& [hkl, row] : rows) {
|
||||
float i_mean, sig_i_mean, f_mean, sig_f_mean;
|
||||
combine(row.Ip, row.sIp, row.Im, row.sIm, i_mean, sig_i_mean);
|
||||
combine(row.Fp, row.sFp, row.Fm, row.sFm, f_mean, sig_f_mean);
|
||||
mtz.nreflections = static_cast<int>(out_rows.size());
|
||||
mtz.data.reserve(out_rows.size() * (has_anom ? 16 : 8));
|
||||
for (const auto& row : out_rows) {
|
||||
mtz.data.push_back(static_cast<float>(row.h));
|
||||
mtz.data.push_back(static_cast<float>(row.k));
|
||||
mtz.data.push_back(static_cast<float>(row.l));
|
||||
mtz.data.push_back(i_mean);
|
||||
mtz.data.push_back(sig_i_mean);
|
||||
mtz.data.push_back(row.Ip);
|
||||
mtz.data.push_back(row.sIp);
|
||||
mtz.data.push_back(row.Im);
|
||||
mtz.data.push_back(row.sIm);
|
||||
mtz.data.push_back(f_mean);
|
||||
mtz.data.push_back(sig_f_mean);
|
||||
mtz.data.push_back(row.Fp);
|
||||
mtz.data.push_back(row.sFp);
|
||||
mtz.data.push_back(row.Fm);
|
||||
mtz.data.push_back(row.sFm);
|
||||
mtz.data.push_back(row.Imean);
|
||||
mtz.data.push_back(row.sImean);
|
||||
if (has_anom) {
|
||||
mtz.data.push_back(row.Ip);
|
||||
mtz.data.push_back(row.sIp);
|
||||
mtz.data.push_back(row.Im);
|
||||
mtz.data.push_back(row.sIm);
|
||||
}
|
||||
mtz.data.push_back(row.Fmean);
|
||||
mtz.data.push_back(row.sFmean);
|
||||
if (has_anom) {
|
||||
mtz.data.push_back(row.Fp);
|
||||
mtz.data.push_back(row.sFp);
|
||||
mtz.data.push_back(row.Fm);
|
||||
mtz.data.push_back(row.sFm);
|
||||
}
|
||||
mtz.data.push_back(static_cast<float>(row.rfree));
|
||||
}
|
||||
mtz.write_to_file(filename);
|
||||
|
||||
@@ -13,12 +13,6 @@
|
||||
namespace {
|
||||
constexpr size_t MIN_REFLECTIONS = 20;
|
||||
|
||||
double SafeInv(double x, double fallback) {
|
||||
if (!std::isfinite(x) || x == 0.0)
|
||||
return fallback;
|
||||
return 1.0 / x;
|
||||
}
|
||||
|
||||
struct BestReindex {
|
||||
gemmi::Op op = gemmi::Op::identity();
|
||||
bool is_identity = true;
|
||||
@@ -150,7 +144,7 @@ double ReindexAmbiguityResolver::ReferenceCC(const std::vector<Reflection> &refl
|
||||
const auto it = reference_data.find(hkl_key_generator(h[0], h[1], h[2]));
|
||||
if (it == reference_data.end())
|
||||
continue;
|
||||
const double x = static_cast<double>(r.I) * SafeInv(r.rlp, 1.0) / r.partiality;
|
||||
const double x = static_cast<double>(r.I) * r.rlp / r.partiality;
|
||||
const double y = it->second;
|
||||
if (!std::isfinite(x) || !std::isfinite(y))
|
||||
continue;
|
||||
|
||||
@@ -108,6 +108,10 @@ std::string RugnuxCommandLine(const ProcessConfig &config,
|
||||
add("--integration-radius", radii.str());
|
||||
if (bragg.GetStillPartiality())
|
||||
args.emplace_back("--still-partiality");
|
||||
// Background trim defaults to 0.10 in the CLI, so emit it whenever the GUI value differs (a
|
||||
// custom fraction, or 0 when the box is unchecked) to reproduce the GUI's choice faithfully.
|
||||
if (bragg.GetBackgroundTrimFraction() != 0.10f)
|
||||
add("--background-trim", num(bragg.GetBackgroundTrimFraction()));
|
||||
|
||||
if (config.rotation_indexing) {
|
||||
if (config.two_pass_rotation)
|
||||
@@ -147,6 +151,8 @@ std::string RugnuxCommandLine(const ProcessConfig &config,
|
||||
args.emplace_back("-B");
|
||||
if (sc.GetPartialityUncertaintyCoeff() > 0.0)
|
||||
add("--partiality-uncertainty", num(sc.GetPartialityUncertaintyCoeff()));
|
||||
if (sc.GetStillsModulation())
|
||||
args.emplace_back("--stills-modulation");
|
||||
// When merging, the CLI skips the large _process.h5 unless asked; emit the flag when it is
|
||||
// wanted so a copied command matches the GUI's "Save _process.h5" choice. (write_merged has
|
||||
// no CLI equivalent - the CLI always writes the .mtz/.cif when merging.)
|
||||
|
||||
@@ -610,12 +610,11 @@ int main(int argc, char **argv) {
|
||||
refine_geometry_disabled = true; // opt out of the stills default-on
|
||||
break;
|
||||
}
|
||||
int n = optarg ? atoi(optarg) : 200;
|
||||
if (n <= 0) {
|
||||
logger.Error("Invalid --refine-geometry value (positive frame count or 'off'): {}", optarg);
|
||||
exit(EXIT_FAILURE);
|
||||
}
|
||||
refine_geometry = n;
|
||||
// Positive frame count fed to the bundle adjust; upper-bounded so refine_frames * 50 in
|
||||
// RefineStillsGeometry cannot overflow int (1e6 is already far past any real dataset).
|
||||
refine_geometry = optarg
|
||||
? parse_number_arg<int>(optarg, "--refine-geometry", logger, 1, 1000000)
|
||||
: 200;
|
||||
break;
|
||||
}
|
||||
case OPT_FORCE_ROTATION_LATTICE: {
|
||||
|
||||
Reference in New Issue
Block a user