4 Commits
Author SHA1 Message Date
leonarski_fandClaude Opus 5 7bb74a9dc8 review fixes: two silent wrong outputs, and one rule for a quantity nobody measured
From four whole-branch code reviews of rc166. Reviewing the net diff rather than
the commits found what per-commit review structurally cannot: a later commit
leaving an earlier one's claim standing, and two cases of a later commit quietly
undoing an earlier one.

THE FITTED RESOLUTION WAS THE P1 CROSS-CHECK'S. The block that merges in P1 to
write <prefix>_P1.mtz saves and restores the error model around itself, because
that merge is not the run's answer. A later commit taught the same function to
report the CC1/2 resolution fit and did not extend the list, so every de-novo
rotation run in a non-P1 group has been quoting FITTED_RESOLUTION - the number
the report itself calls the one to quote - from a merge with n_ops times the
unique reflections at a fraction of the multiplicity.

AND IT ROUND-TRIPPED THE SPACE GROUP THROUGH ITS NUMBER. A number names only the
reference setting, which stopped being enough when the search learned to adopt
P 1 1 2(1) or I 1 2 1. Everything written after that block - the unmerged MTZ
included - therefore carried the reference setting while the merged file carried
the adopted one: two files describing one dataset in two different settings. It
now carries the group.

A PONI CANNOT STATE A MIRRORED OR QUARTER-TURNED DETECTOR, and the fits became
orientation-aware on this branch while the writer did not. It wrote five numbers
that silently described a different geometry from the one measured; it now
refuses, and says the JSON beside it has the full one.

A REFUSED FIT'S ERROR BARS COULD BE HANDED BACK AS AN ACCEPTED GEOMETRY'S.
RingOptimizer::Run writes its uncertainty only where the solve is usable, and
CalibrateFromSpots runs it twice - tilt free, then tilt pinned. A failed second
fit kept the first's sigmas, valid flag and all. It is cleared on the way in.

ONE RULE FOR NOT MEASURED. The report had four conventions for it and printed the
same missing quantity two ways on adjacent lines: SIGANO as the literal "nan" and
CC_ANOM by absence, for a Friedel-merged run that split no Bijvoet pair - which is
the default. A quantity a run did not measure now writes no key, and the shell
table's dash follows the same rule rather than a 0.0% that reads as a measured
total failure. ANISOTROPY_D_MIN_BEST also stops printing nan when only its first
principal direction is unmeasured.

Four claims that a later commit made false are corrected where they stand: the
merge header promising an order-independence the balancing rule gave up, the
reference page arguing against CCanom 28 minutes before it shipped, the screw
threshold whose "three dead reflections clear it" the evidence floor caps at 19.2
nats, and a shell comment calling equal width in 1/d^2 equal volume.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-02 17:27:56 +02:00
leonarski_fandClaude Opus 5 0c3e1462ef merge: the completeness denominator is the declared range, not the surviving one
CalcPossibleReflections was handed d_min/d_max derived from the reflections that
came out of the merge, so a loss at either extreme took the numerator and the
denominator with it. At the high end that is right: d_min is the finest d reached
anywhere and the denominator is the full sphere down to it, so anisotropic loss
shows. At the low end it was a tautology - d_max was the coarsest reflection that
happened to survive, so anything the beam-stop shadow mask (on by default), a
detector mask or the low-resolution limit itself removed left the denominator
along with the data and could not be reported as missing.

Both statistics paths now bin, and count, between the DECLARED low-resolution
limit and the finest d reached: MergeOnTheFly::MergeStats (stills) and
RotationScaleMerge::MergeAndStats (rotation). The grid and the denominator keep
sharing their bounds, so no possible reflection falls outside a shell. An
undeclared low limit is the whole sphere - 1/d^2 down to 0 - spelled as an
infinite d_max, which ResolutionShells already handles and which gemmi's
for_all_reflections special-cases; the change therefore reads correctly whether
or not the 50 A default stays. The innermost shell keeps a finite d_max label,
falling back to the coarsest reflection measured when the bound is infinite.

This makes the shell boundaries the ones the integration document already claims:
XDS lays its nine 1/d^2 bins between INCLUDE_RESOLUTION_RANGE's two values, not
between the extremes of the surviving data, and counts POSSIBLE against the
declared low limit - which is why its innermost shell reports the beam stop's
loss. Verified against a CORRECT.LP: all nine boundaries reproduce to the printed
precision from the declared 50 A, and not from the coarsest observed reflection.

Measured on stored merges of seven rotation datasets, small-molecule and protein,
re-scaled with --mode scale: the overall denominator moves by 0 to 2 reflections
out of 70,000-100,000, because on every one of them the coarsest reflection the
declared limit allows was itself measured - the corpus has no dataset whose stop
eats a whole low-resolution class. What does move is the shell grid: the
innermost boundary shifts by 0.1-0.4% in d (e.g. 7.21 -> 7.22 A), which changes
the innermost shell's counts by up to a few per cent and its R_meas by around
0.1 percentage points. Stored battery baselines for rmeas_lo must therefore be
regenerated, not compared across this commit.

Two decisions read merged completeness (the two-pass wrong-cell guard, which only
fires above 100.5% and only under -S); a larger denominator can only lower the
figure, so the guard can fire less often, never more.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EFEJG6WBQv8th4UJFNe53N
2026-09-02 16:39:40 +02:00
leonarski_f cc3eb8352c v1.0.0-rc.148 (#58)
Build Packages / Unit tests (push) Skipped
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 9m28s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 10m9s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 9m47s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 10m58s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 11m39s
Build Packages / build:rpm (rocky8) (push) Successful in 11m43s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 12m59s
Build Packages / Generate python client (push) Successful in 35s
Build Packages / Build documentation (push) Successful in 59s
Build Packages / Create release (push) Skipped
Build Packages / build:rpm (ubuntu2204) (push) Successful in 11m48s
Build Packages / build:rpm (rocky9) (push) Successful in 12m32s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 10m24s
Build Packages / XDS test (durin plugin) (push) Successful in 7m35s
Build Packages / XDS test (neggia plugin) (push) Successful in 6m50s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 7m40s
Build Packages / DIALS test (push) Successful in 11m19s
This is an UNSTABLE release. The release has significant modifications for data processing - in case of troubles go back to 1.0.0-rc.144.

* jfjoch_broker: Improve azimuthal integration (add <I^2> calculation)
* jfjoch_broker: Fixes around indexing, aiming to handle multi-lattice crystals (work in progress, it is not fully integrated)
* jfjoch_writer: Save mean(I), stddev(I), and count(I) for each azimuthal bin

Reviewed-on: #58
2026-06-08 08:30:35 +02:00
leonarski_f 061152279c v1.0.0-rc.91 2025-10-20 20:43:44 +02:00