1317 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
leonarski_fandClaude Opus 5	1bc2ca9125	rugnux: do not re-find the rotation spots just because --detect-ice-rings was named ... First-pass rotation indexing reuses the spots the acquisition wrote, and does NOT re-mark them - their ice flags come from the file. So --detect-ice-rings can only invalidate those spots when it asks for something the file did not do. It was flagged as a spot-finding option, which forced reuse_rotation_spots off whenever it appeared, so naming it swapped the acquisition's spots for this program's own and moved the first-pass lattice by itself. Measured over the 37-crystal rotation battery, passing the SEMANTICALLY NULL --detect-ice-rings=on to files that already carry detect_ice_rings=1 changed the merged data on every crystal, sent one crystal's ISa from 1.66 to 0.38, and lost MyoB_13 to indexing failure outright. Both arms of any A/B on the flag therefore moved for a reason that had nothing to do with ice, which made the flag impossible to test. Re-find only when the requested value differs from the file's, and say so when it happens. A file with no key at all counts as "did not mark", which is what its stored spots show - such a dataset carries no per-spot ice flags to reuse. Verified over the battery, with the merge mask and the radial background pinned off so this is the only variable. --detect-ice-rings=on on the 36 keyed crystals: all 36 log "using the spots stored in the file", the re-finding line appears nowhere in the arm, unique counts are identical on every crystal, the largest mean |dI|/sigma is 3.4e-5 against a repeat-run floor of 9.3e-5, and not one of R_meas, CC1/2, CC1/2_hi, ISa, completeness, SigAno, d_min or space group differs anywhere. MyoB_13 indexes again. The one file carrying no key reuses under =off and re-finds under =on, as it should. --detect-ice-rings=off still re-finds, since it does differ from those files, and two crystals still fail to index there. That is not this change: a control that re-finds with ice marking ON indexes both. With the marking off, ice spots are no longer ordered last, so they consume the --max-spots budget and the first pass collapses. With the confound removed the flag can finally be measured, and on a comparison whose spot source is identical on both arms it is clearly worth having - though the win is at INDEXING rather than at the scale fit. Three crystals are saved outright (one would otherwise collapse to P1 at CC1/2 19%, one loses half its completeness and its screw axis, one loses its F-centred cubic lattice), two more only index with it on, and the remaining eight gate-fired crystals differ by well under 1% in R_meas and CC1/2 in both directions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-06 20:18:17 +02:00
leonarski_fandClaude Opus 5	f0cdb027e1	Ice: default the merge mask off, gate the radial background on smooth ice, and pick detection by geometry ... Three defaults, each settled by measurement rather than by argument. The arbiter throughout is structure-referenced - anomalous peak height where a crystal can carry it, and otherwise the agreement of the ice bands with a fixed external model against resolution-matched DECOY bands carrying no ice. The band-versus-decoy contrast is used because R-free here tracks completeness, and every one of these switches moves completeness. The damage is real and it localizes: over the rotation battery the ice bands' excess amplitude reaches +9.6% on a smooth-ice crystal and +35% on the worst, while a clean control sits at +0.6% (z +0.45). On the worst crystal, nine of the ten largest excess peaks in a q scan land on hexagonal ring positions. Turning ice handling off leaves the contrast unchanged and forcing it on a clean crystal does not create one, so it is the ice and not the machinery. MERGE-TIME RING MASK -> OFF. It deletes reflections, which no other program does by default - AIMLESS, DIALS, xia2, XDS and CrystFEL all keep ice-band reflections in the merge and exclude them only from the model fit; autoPROC is the sole exception. On the one battery crystal where the mask fires and an anomalous arbiter can score it, dropping the band moved the mean peak height at the known sites by -0.001 +- 0.018 sigma, 2% of the site height, while removing 1149 unique reflections whose mean I/sigma was 3.62 against the dataset's own 3.05 - better than average data - and costing 17 completeness points in that shell. It fires on 5 of 37 crystals, changes no space group, and those 5 disagree in sign: it clearly helps the two most heavily iced, is a wash on two and costs a third. So it stays as a switch, worth setting by hand on a badly iced crystal where it shows in the high shell, but it is not a default. RADIAL BACKGROUND -> AUTO, gated per image. The correction models the background as a function of radius alone, and that is exactly when it works. On a crystal with pure smooth powder ice it removes 43% of the bands' excess amplitude, with the improvement 7x larger inside the bands than outside; on a crystal whose ice is discrete crystallite spots - no smooth ring to model - the excess amplitude GREW by half; on clean data it is inert to four decimals. The two ice channels already separate those morphologies, so --background-radial takes on|off|auto and auto applies it to an image when that image's peak-excluded score reaches --ice-min-score. Auto never engages without such a score, because the plain profile carries the Bragg peaks and cannot support an absolute threshold. Per image rather than per run, and that was tested rather than assumed: the gate fires on 100% and 94% of frames on the two crystals that want it, and on 1.5% of frames - 32 blocks, 23 of them single frames - on the textured-ice crystal. A seam statistic against off + f*(on - off) is null on both mixed runs, every merge statistic is bracketed by the pure arms, and the textured crystal's auto arm lands on `off` rather than on `on`'s harm. A run-level gate would need the score before the pass that integrates, i.e. rotation-only plumbing, and buys nothing measurable. The kernel was already built unconditionally, so flipping the flag per image is free - except on the GPU, where the launches were gated on a construction-time n_rad. That is why the buffers are now allocated whenever the correction could run, and Run() decides per image. DETECTION -> the geometry's default when the file is silent: on for rotation, off for stills, with the command line and then the file taking precedence. A rotation sweep sits on the same rings for the whole run, so ice there is a coherent systematic and the presence gate keeps it inert on a clean crystal; a serial stills run has too few spots per image to spend any on flagging. The master file's key is kept as written rather than collapsed to a bool, so "the file said nothing" is distinguishable from "the file said no" - it used to fall silently to off, taking the exclusion from the scale fit with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-06 19:06:30 +02:00
leonarski_fandClaude Opus 5	06b8c8ed66	Merge statistics: count the observations the merge kept, not the ones it walked ... Whenever the merge-time ice-ring mask dropped a band, the per-shell observation count and hence the reported multiplicity were wrong. On one crystal the lowest resolution shell read 40780 observations over 1932 unique reflections - 21.1x - where the truth is 27007 and 13.98x, and the overall redundancy read 12.52 against 12.29. Only counts were affected: intensities, sigmas, R_meas, CC1/2, completeness and ISa were right throughout, because a masked group carries merged_I = NaN and never enters those sums. It looked like double counting and was not - it is a MOVE. Two independent faults, both in three lines: total_obs rides on the R_meas re-walk, whose filter deliberately ignores the ring mask (and, on a search pass, the ice flag) so that R_meas is computed on the same reflections either way. RmeasUsable therefore differs from MergeUsable by exactly those two tests, and the observations they admit were being counted against a `unique` that excludes them. On the GPU path that count is binned by the GROUP's resolution, and a group every one of whose observations is masked never has one written - acc[g].d stays NaN. ResolutionShells::GetShell(NaN) then returned shell 0 rather than nothing: NaN fails both bound comparisons, falls through to the arithmetic, and static_cast<int32_t>(NaN) is INT_MIN, which the clamp maps to 0. So the masked ring's observations were re-labelled into the lowest-resolution shell, four shells from the ring they came from. The two paths disagreeing on the same run is what settled it: with the mask on, the GPU statistics gave shell 0 = 752 and the CPU statistics 423, while the merged intensities were identical. Count the merged population instead - acc[g].nh, which the merge already accumulates per group - and guard the CPU increment with usable_merge. The rnusable skip stays: any group present in the merged output has at least one observation passing MergeUsable, and MergeUsable is a subset of RmeasUsable, so it cannot drop a group that contributes to `unique`. With the mask off and for_search false the two predicates are identical, so this is provably inert on every shipped configuration - demonstrated on four configurations, including one where ice handling is active but the mask does not fire: the statistics blocks are unchanged. (The reflection lists differ in the last ulp on 3-12% of lines, but so do two runs of the same binary; that is the known rotation nondeterminism, and the statistics block is what is stable.) The NaN guard also removes a silent contamination nobody was looking for. Four call sites validate a resolution with `d <= 0`, which NaN passes: the Wilson-B fit and per-shell <I/sigma> (CalcISigma), the per-image resolution plot (SpotUtils) and the shell Wilson prior (FrenchWilson) were all binning non-finite d into their lowest-resolution shell. French-Wilson now falls back to the global mean rather than to that shell's, which is the worst prior available. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-06 19:06:00 +02:00
leonarski_fandClaude Opus 5	17eb6ef091	Post-refine: report the goniometer rotation scale it already fits ... A stage that turns further than commanded is invisible in the file, because the stored omega values ARE the commanded ones - both XDS and rugnux then read the discrepancy as the crystal drifting. Measured on one dataset in 37, a ~1.2 % over-rotation costs it unique 9.9k -> 29k and CC1/2 68 -> 98 % when corrected by hand. No new degree of freedom is added, because the one needed is already there and being thrown away: step A's residual rotates by -angle_rad * axis[] with axis an UNNORMALISED 3-vector, so the length it fits IS the factor by which the stage actually turned. GoniometerAxis::Axis then normalises it away (with the `increment *= len` line sitting commented out). This only reports it. Guarded by the same cross-validation that gates the cell move - a fold that merely soaked up noise cannot raise the flag - and by a 0.5 % tolerance, which is where a direct scan of this factor puts 36 of 37 datasets (all at exactly 1.0000). The known fault reads 1.00604 and warns; clean controls read 0.99958 and 0.99954. It UNDER-reads the true magnitude: the fit only sees reflections already indexed at the nominal angle, per-frame orientation refinement has absorbed part of the error, and the axis components are bounded. Treat it as a detector, not a calibration - nothing here corrects the data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-06 16:17:23 +02:00
leonarski_fandClaude Opus 5	61a7c91b90	Ice: detect it on two channels, and only handle it when it is there ... The per-image ice score was read off the PLAIN azimuthal profile. That profile is a per-ring mean, so a few strong Bragg reflections landing in a ring's q bin lift it exactly as ice would. Measured over 37 rotation crystals, that did not merely add noise - it INVERTED the metric: the two highest-scoring crystals had no ice at all (4.23 and 4.06), while a clean control read 1.57. A decoy null - the identical statistic evaluated at q positions where hexagonal ice cannot be - reaches 1.51 at its 99th percentile and 2.70 at its maximum, so that metric cannot support any absolute threshold whatsoever. The adaptive spot finder already computes the right input for its own threshold: a sigma-clipped per-resolution-ring background, in the same bins. A powder ring is azimuthally smooth and survives the clip; Bragg peaks do not. On the clipped profile the clean population tightens to 1.00-1.22 and the crystals with confirmed ice sit at 2.08-2.37, against a decoy null that never exceeds 1.29. That channel is blind to one thing: ice in large crystallites diffracts as DISCRETE spots and leaves the radial profile flat. So a second channel counts found spots on the rings against the same q width of ice-free flanks beside them. The two barely overlap - the smooth-ice crystals read 2.1-2.4 / ~1.0 and the textured ones ~1.1 / 3.8-17.6, while a clean crystal reads 1.04 on both. Both are then used as a GATE (--ice-min-score 1.5, --ice-min-spot-ratio 2.0, both calibrated on the battery, 0 disables): the eleven fixed hexagonal bands cover 16-26 % of the unique reflections at typical resolutions whether or not the crystal has ice, so flagging, the exclusion from the scale fit and the merge-time CC1/2 ring mask are now all skipped when neither channel sees any. The gate is applied in the full pipeline and in --scale, which reads the stored per-image values back out of the _process.h5. Also fixes the merge-time mask's control: the shoulder now excludes reflections that are themselves on an ice ring. The rings are not evenly spaced - 1.947/1.916/1.882 A sit 0.05-0.06 apart in q - so for those three the [w,3w) shoulder landed squarely on the neighbours and the test compared ice against ice. Measured, that is the only thing this changes: it removes firings on those three rings and leaves every other firing's CC pair identical to three decimals. And the online ice half-width, which was 0.02 in the API against 0.03 offline, so the same data got a narrower band online than the measured ~0.06 ring FWHM justifies. Battery (37 rotation crystals, against the previous behaviour): space groups 34/37 in both and NO crystal's space group changes; 6 crystals gain unique reflections, 1 loses. Best of them gains 7082 unique reflections with R_meas 16.0 -> 14.3, CC1/2 95.9 -> 97.3 and ISa 13.7 -> 19.0; another goes R_meas 54.9 -> 42.9, CC1/2 84.0 -> 90.4, ISa 3.9 -> 5.5; a third reaches CC1/2 99.4 from 95.7 at an unchanged reflection count. The one crystal that loses reflections improves on both R_meas and CC1/2. Not done here: the ScanResult/API/plot-type/frontend/viewer layers for the new spot_count_ice_control (they need the OpenAPI regeneration). Message, CBOR, HDF5 write/read and the receiver plots are. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-06 16:17:23 +02:00
leonarski_fandClaude Opus 5	0e23fd3ab9	Bragg integration: propagate the background-estimate uncertainty, add an opt-in radial background correction ... Two independent pieces in the same code path. The background-estimate variance was never propagated. A reflection's background comes from a finite ring of n_b pixels, so subtracting it adds var(B)/n_b per signal pixel - sqrt(1 + n_d/n_b) = 1.109 with the shipped stencil. Both engines omitted it, which is exactly the 1.11-1.19 gap measured between the off-ring scatter and the reported sigma. Three lines each; it affects every dataset, not only iced ones. The radial correction is new and OFF by default (--background-radial). The signal disk and the background ring are concentric, so for any background LINEAR in position <B>_ann == <B>_disk identically and a plane fit buys nothing; the leading error is the CURVATURE of the radial background, which on a sharp ice ring reaches +26 counts on a single reflection. Since every reflection uses the same stencil, that error is a fixed kernel over radial offset - one short dot product per reflection and no extra pixel reads. Validated on empty apertures before any C++: mean |bias| over 9 bands / 3 crystals 4.33 -> 0.79 counts with the scatter unchanged. Three things it cost a battery each to learn, all now in the code: - the radial curve must be accumulated from CLIPPED annulus pixels, inside the clip pass, or it carries neighbour tails and zingers (so it is inert under --integrator boxsum, which has no clip pass); - the GPU version was a 1.8x slowdown from atomicAdd contention on a small radial array - staged in shared memory per block it now costs nothing measurable; - it is battery-NEUTRAL as a default, because the reflections whose bias it fixes are the ones the ice handling already excludes. Hence off by default. CPU/GPU parity extended with two radial sections: 9002 assertions. Also fixes a latent French-Wilson quadrature collapse: j_max = I + 8 sigma on a fixed 400-point grid degenerates to a single cell once sigma >> 50 <I>, giving F = 0.1 sqrt(sigma) with sigmaF -> 0. Harmless today, but any sigma-inflation scheme detonates it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-06 15:44:13 +02:00
leonarski_fandClaude Opus 5	52f0e58cae	rugnux: do not smooth un-indexed frames into the per-frame geometry ... SmoothGeometry de-rotates each frame's lattice to a common reference, averages in frame order and rotates back. A frame that never indexed keeps a default-constructed CrystalLattice whose vectors are all ZERO - and zero is finite, so the isfinite guard let it through. Those zero vectors were averaged into their neighbours' smoothed orientation, pulling it toward the origin, and they were scored in the leave-one-out cross-validation that picks the smoothing window. On a crystal where 374 of 900 frames fail to index, the effect on the window choice is not subtle. Measured: before n_scored 900 (only 526 indexed) CV score ~504-542 A^2 window +-12 after n_scored 516-526 CV score 0.160-0.175 window +-2 The score was inflated ~3000x and the choice among windows was noise. It settled on +-12 frames - 9.6 degrees of goniometer rotation - on a crystal whose orientation genuinely drifts by ~8 degrees over the sweep, so every partial's delta_phi was recomputed from an orientation averaged across that drift. Require a real cell. Exactly inert when every frame indexes, and no threshold is touched. The crystal that exposed it goes P1 -> P2_1, observations 60107 -> 77021, completeness 64.1% -> 93.0%, multiplicity 1.10 -> 2.0, CC1/2 70.0% -> 84.9%, R_meas low shell 37.3% -> 22.1%, and its 2-fold operator CC 0.330 -> 0.669, comfortably clear of the 0.5 gate. Battery over 37 crystals: space groups 33/37 -> 34/37, and that crystal is the ONLY flip - no losses. Another crystal is rescued from near-total collapse (4402 -> 139213 observations) because the two-pass "going back to the header geometry" fallback stops firing. Anomalous peak height +0.043 +- 0.022 sigma over 7 crystals, so the background clip's gain is intact. Merged quality is otherwise neutral (CC1/2 6 better/6 worse, R_meas_lo 9/6) with observations up on 18 crystals. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-06 01:21:21 +02:00
leonarski_fandClaude Opus 5	d80f48e6bd	rugnux_anomalous.py: read ANODE peak labels for any scatterer, not just sulfur ... ANODE names its ranked peaks after the SFAC element it was given, so the labels read S1, S2, ... on a sulfur case but FE1, MN1, ZN1, SE1 as soon as the model carries a heavier scatterer. The parser matched "S" followed by digits, so on any such dataset it dropped the ENTIRE peak list and the dataset failed the gate as "no peaks" - however strong its signal actually was. One heme case reported no peaks when its true top peak is 13.79 sigma at 3.02x the off-site floor. The bug is silent and it mis-gates exactly the datasets most likely to widen the arbiter set, since a heavy scatterer is what makes a weakly diffracting crystal usable as an arbiter in the first place. Byte-identical on the standing all-sulfur set, verified by diffing the gate table before and after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-06 00:20:41 +02:00
leonarski_fandClaude Opus 5	227f1bf1b4	Add rugnux_anomalous.py: judge partiality changes by anomalous peak height ... A change that touches partiality - a mosaicity estimator, a rocking-curve model, a background change - cannot be judged by the statistics we normally reach for, and this was learned the expensive way. ISa is anti-correlated with external accuracy and is the largest mover of any statistic; last-shell R_meas moves with its denominator, i.e. the wrong way by construction; `--model` R-free tracks its own zero-information floor, which shifts ~22x more than R-free itself over the same sweep; and per-shell agreement with XDS is biased, because XDS never divides by partiality, so "divide less" moves us toward it mechanically - measured to put the optimum ~1.4x too low. Anomalous difference density at known scatterer sites has none of those problems. It is read in units of the map's own sigma, so the uniform intensity rescale a partiality change produces cancels exactly, and it is referenced to the structure rather than to another program's partiality model. The script runs SHELXC + ANODE per arm against a model placed ONCE and then held fixed, and reports the mean site height, the off-site noise floor, and the paired per-site change between arms. Numeric arm labels turn a set of arms into a curve with a per-dataset optimum. The dataset table lives outside the repository, as rugnux_vs_xds.py already does for the battery, because dataset and sample identities are not committed. It reproduces the measurements it was built from: all nine points of three pooled curves, every per-crystal optimum, the site heights, the paired t statistics, and the adversarial control in which a model refined against the worst arm reproduces the curves to <=0.005 and the same optimum on 4/4. Four things the ad-hoc scripts it replaces got wrong, all now handled: * Keying sites on the ANODE atom label silently drops an alternate conformation sharing that label - one dataset class has 18 sulfur sites, not 17, and the uncorrected mean read 15.03 against a true 14.49. * The off-site floor skipped any peak within 1.0 A of ANY atom, so a ripple sitting on a light atom was not counted as background; requiring 1.5 A from an anomalous scatterer raises one floor from 7.12 to 9.48 sigma. * Special-position peaks are Fourier ripples, not background. Excluding them is load-bearing on 3 of 9 datasets and they are now reported in their own column rather than dropped silently. * Enantiomorph care turned out to be unnecessary - passing the merged file's screw label to ANODE while the model sits in the other hand gives byte identical peaks. What does matter is the pair whose absences are identical, I23 vs I2_13, which phaser's automatic hand test does not cover. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-05 22:52:39 +02:00
leonarski_fandClaude Opus 5	24ac1a90fc	rugnux: name every space group the data cannot separate in the summary ... The search already knows when several groups share an absence pattern - it scores them identically, marks them all in the candidate table and prints "Best space group: I23 or I213 (indistinguishable from these data)". The one-line summary then dropped that and reported only the representative, so the run's headline answer claimed a decision the diffraction had not made. Carry the alternatives through to the summary. It already has them: ProcessResult holds the whole SearchSpaceGroupResult. Space group: I23 (No. 197) or I213 (No. 199) - indistinguishable from these data Some of these pairs are enantiomorphs (P4_1 vs P4_3), where the choice needs phasing or anomalous signal. Others are not, and are worth naming because they surprise: I23 vs I2_13 and I222 vs I2_12_12_1 differ only by a screw whose condition h00: h=2n is ALREADY implied by the I-centering condition h+k+l=2n, so the screw has no observable signature at all. Checked over 35936 reflections with gemmi, the two absence patterns are identical - not nearly, but exactly. Of the 65 chiral space groups, 13 classes are indistinguishable this way, the largest being the four-way P3_112 / P3_121 / P3_212 / P3_221. The representative stays the lowest space-group number, which is why a cubic insulin comes out I23 where the deposited convention is I2_13. That choice is a convention and the summary now says so instead of implying it was measured. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-05 22:20:43 +02:00
leonarski_fandClaude Opus 5	b22e1b6822	rugnux: raise the ice-ring mask margin to the measured null ... The mask drops a hexagonal-ice ring when its merged half-set CC1/2 falls a fixed 0.05 below its resolution shoulders. That margin is not a significance level: at the populations these rings actually have, 0.05 spans 1.1 to 7.3 sigma across firings, and a nominal Fisher-z error understates the real scatter of these heavy-tailed intensities by ~2.7x, so the null has to be measured rather than derived. Measured it with decoy bands - the identical ring/shoulder statistic evaluated at q positions carrying no ice ring - over the 37-crystal rotation battery: the gap's empirical null is p95 +0.032, p99 +0.095. So 0.05 sits near the 96th percentile, about 4% of ice-free bands clear it, and roughly half the 22 observed firings are indistinguishable from bands with no ice in them. The firing gaps are continuous, not bimodal, with 12 of 22 in [0.05, 0.10). Raise it to 0.10, the 99th percentile of that null. Firings 22 -> 10, crystals 12 -> 5, decoy false-positive rate 3.4% -> 0.8%. An independent check against XDS - which integrates through ice rings and so measures exactly what we delete - agrees: of the firings with a usable comparison, 9 true / 9 false becomes 7 true / 2 false. Battery: space groups 34 OK / 3 DIFF, the same three crystals as baseline, and no other discrete decision changes on 37/37. The heavily iced crystal keeps all five of its rings and its CC1/2 of 96.6; eight others recover 3.9-11.9% more unique reflections and up to 10.4 completeness points. Cost is CC1/2 -0.84 on one crystal, -0.35 on another, and agreement with XDS on the common reflections worse by a median 0.0004. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-05 19:00:10 +02:00
leonarski_fandClaude Opus 5	bb7df09086	rugnux: separate the merge-time ice-ring mask from ice detection ... --detect-ice-rings did two unrelated jobs at once: flagging ice spots so indexing de-prioritises them and keeping ice reflections out of the scale fit, AND gating the merge-time mask that drops a decorrelated ice ring and re-merges. Turning it off to de-confound a merge-stage experiment therefore also changed how the data were indexed - measured, that breaks indexing outright on two of the 37 rotation battery crystals - while leaving it on lets the mask land differently between two arms of an experiment and contaminate the comparison (measured on up to 19 of 37 crystals in response to a small intensity change). Add --ice-ring-mask[=on|off], default on, gating only the merge-time mask. Verified with =off: ice-spot flagging and the scaling exclusion still log and still apply, no mask line, no second merge, and the first error model is bit-identical to the =on arm. The full pipeline and the offline --scale path reach the same verdict on the same data, as they must. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-05 19:00:10 +02:00
leonarski_fandClaude Opus 5	09fb8e0306	Bragg integration: clip the background ring high side instead of trimming it ... The r2..r3 background ring was averaged with a 10% SYMMETRIC trimmed mean. A symmetric trim is not a consistent estimator of the mean of a right-skewed (Poisson) sample: on a clean Poisson ring it sits ~0.1 ct/px BELOW the true mean at every level, and with ~50 signal pixels in the r1 disk that under-subtraction adds ~5 counts to every partial on every frame. Measured two independent ways on four rotation datasets - stored background_mean against a plain ring mean over the same pixels on reflection-free frames, and directly on apertures that provably hold no reflection. Empty-aperture pedestal, counts: plain mean -0.03..-0.20, 10% symmetric trim +5.05..+6.34, 4 sigma clip +0.02..+0.54. Replace it with a high-side-only sigma clip at mean + n*sqrt(mean), n = 4 for monochromatic data. It rejects the same one-sided contamination the trim was there for - better, in fact: a 40 px neighbour core at +100 ct shifts the trim by +10.1 ct/px, because a symmetric trim collapses once contamination exceeds ~10% of the ring, versus +0.009 ct/px at 4 sigma. False rejection on a clean ring is 0.04-0.39%. Broadband data keep their tuned 3 sigma clip unchanged. The trim stays reachable with --background-trim for back compatibility; setting either estimator clears the other, so they can never stack. --integrator boxsum does not take the clip (matching what the shipped clip already did), so it now uses the plain ring mean unless --background-trim is given. The intensities get measurably more accurate: per-shell agreement with an independent processing of the same images improves on 14 of 16 crystals (weighted -0.0347, outermost shell 12/4), the outermost-shell R_meas NUMERATOR - absolute scatter, not a denominator effect - falls 13.5% median on 16/5, and CC1/2 in the outer shell improves on 14/7. EXPECT <I/sigma> TO FALL AND EDGE R_meas TO RISE. Both are inflated by information-free counts, so both get worse when the bias is removed; neither is evidence against this change. That fingerprint is exactly how the trimmed mean was accepted in the first place. Known cost: over the 37-crystal rotation battery the de-novo space-group count goes 34 OK / 3 DIFF to 33 / 4. The single regression is a two-lattice crystal whose merge fails the absolute-sanity gate under either background (R_meas 63.5%, CC1/2 72.2%) and which carries an unresolved indexing ambiguity on the very operator being scored, so its operator CC is diluted by construction. No other crystal changes space group, and twin protection is not weakened - the H-ratio veto that refuses genuinely twinned crystals gets MORE decisive (1.63 -> 1.84, 2.83 -> 3.99). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-05 18:03:11 +02:00
leonarski_fandClaude Opus 5	9549caf82b	docs: do not run the whole test suite locally, and do not push ... Running bare `./jfjoch_test` costs far more time than it is worth on a developer machine, and CI runs the full suite on every push regardless; the useful local run is the cases or tags covering the code that changed. Same reasoning for the 37-crystal rotation battery - it belongs on changes that plausibly move merged results, not on every edit. Also record that pushing is the maintainer's decision, not part of "make the change". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-05 16:51:56 +02:00
leonarski_fandClaude Opus 5	8ecd126e93	rugnux: stop the space-group search starving on a low-ISa merge ... The correlation stage kept only reflections with I/sigma >= present_i_over_sigma (3.0). That statistic is taken on the P1-MERGED intensities, whose sigma is floored at b|I| (Merge.h, SigmaWithSystematicFloor) so that ISa = 1/b is the asymptotic I/sigma ceiling - no reflection in a merge can read above it. Verified over the rotation battery: max I/sigma equals 1/b on every merge. So a fixed cut is not a per-reflection test at all. Every reflection sitting at the floor reads 1/b exactly, however strong, and on a merge whose ISa falls below the cut NOTHING passes: every operator is left with no pairs, its CC is NaN, and the point group collapses to 1. The predicate "search-merge ISa < 3" identifies the affected crystals exactly. It is latent today - no crystal in the battery starves on the shipped integration background - but it fires on four as soon as an additive intensity bias is removed, and it is not a data-quality verdict: the crystals it silences have final merges at ISa 19-22 while their low-multiplicity search merge sits at 3.5-4.0, just above the cut. Cap the cut at the merge's own I/sigma quantile so the correlation stage always keeps at least its strongest quarter. A no-op wherever the fixed cut already keeps that many - the cut stays exactly 3.000 on healthy merges. Battery unchanged at 34 space groups matching XDS / 3 differing, with merged observations identical to 0.000% on all 37 crystals. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-05 16:48:40 +02:00
leonarski_fandClaude Opus 5	fb55645b81	Revert "rugnux: fit the profile radius from the strongest spots too" ... Reverts the profile-radius part of 457b1bfd1; the comparison-script and mosaicity-column changes from that commit are kept. The cap was validated on the rotation battery, which cannot test it: the profile radius feeds `ewald_dist_cutoff` in IndexAndRefine, and that is read only by the STILLS predictors (BraggPrediction/BraggPredictionGPU). The rotation predictors gate on the mosaicity window instead and never look at it. So "no space-group changes, 36 of 37 crystals bit-identical" showed the quantity is inert for rotation, not that capping it is safe - and the one regime where it does act was never exercised. Validating it needs the serial-stills battery, which is a much larger exercise. Until then the arbitrary constant is not worth carrying in a code path nobody measured. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-04 21:35:17 +02:00
leonarski_fandClaude Opus 5	5eb386e333	docs: describe the per-frame geometry smoothing ... Goes in §10.3 next to the per-frame scale and mosaicity smoothing, since it is the same mechanism applied for the same reason, and trims the changelog line to one sentence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-04 20:23:19 +02:00
leonarski_fandClaude Opus 5	017f64690c	rugnux: smooth the per-frame geometry before scaling ... Geometry is re-refined independently on every frame, against that frame's spots alone - as few as a dozen on a sparse crystal, where XDS fits its equivalent to about sixty times more data. Measured over ten datasets the per-frame orientation carries two components: a slow drift that is real, with rugnux and XDS agreeing to R^2 0.83-0.88 on the two crystals that genuinely slip by 1.5 and 0.54 degrees, and a fast jitter that is fit noise, scaling with spots-per-frame at exponent -0.79 where counting noise alone would give -0.5. The jitter is worth 1-8% on merged intensities, 24% on the sparsest crystal. It cannot be fixed by refining less. Turning per-image refinement off entirely loses six space groups and a whole crystal, and even a 624-spot-per-frame crystal collapses; dropping the beam-centre terms holds the space groups but is worse on 31 of 37 crystals. The freedom is earning its keep, so keep it and suppress only the band that cannot be physical - a crystal does not re-orient and snap back from one frame to the next. So smooth the orientation in frame order after integration and recompute each partial's delta_phi, and hence its partiality, from the smoothed lattice. Batching at integration time was not an option: frames are processed independently and the online path depends on that. This runs before the GPU upload, so the device path picks it up with no separate kernel. The window is chosen per dataset by leave-one-out cross-validation, because the two components vary far too much for one number - drift spans 0.018 to 1.288 degrees and jitter 0.005 to 0.221, so any fixed window over-smooths one crystal while under-smoothing another. Chosen windows range from +-1 to +-20 frames. It is capped: cross-validation scores how well neighbours predict a frame's orientation, which on a barely-drifting crystal keeps improving with width, but the per-frame fit is also absorbing a real per-frame systematic and smoothing too wide destroys it - uncapped, one crystal chose +-60 and lost 16% of its ISa. Battery over 37 crystals: space groups unchanged at 34 matching XDS, R_meas better on 31 and worse on 6, low-resolution R_meas 30/7, ISa 26/10, high-resolution CC1/2 23/12. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-04 20:06:41 +02:00
leonarski_fandClaude Opus 5	457b1bfd1d	rugnux: fit the profile radius from the strongest spots too ... Same defect as the mosaicity in 2c94f3013, in the same file's sibling fit. The profile radius is an RMS of the excitation error over whatever spots were kept, and weaker spots sit further off the Ewald sphere, so it grows with the depth of the list: measured over a 150 -> unlimited spot budget it rises ~60%, and on a clean dataset as much as on a hard one, so this is general rather than something one awkward crystal provoked. That made it a function of --max-spots, which is an indexing budget, rather than of the crystal. Its one consumer treats it as a membership gate (ewald_dist_cutoff is twice it) where reflections at the cutoff carry near-zero partiality and are removed downstream anyway, so the integrated data barely notices: with the mosaicity already pinned, the partial count moves 0.2% across a 34% change in the radius. It is also reported per image as a diagnostic, though, and a number that slides with an unrelated setting is misleading to anyone comparing two runs - and it would stop being benign the moment anything used it as a width rather than a gate. Battery over 37 crystals: no space group changes, 36 of 37 bit-identical, no failures, one crystal marginally better. Also in the comparison script: report XDS's mosaicity next to rugnux's. XDS has two and they are not interchangeable - CORRECT.LP's REFLECTING_RANGE_E.S.D. is post-refined, while INTEGRATE.LP's per-batch SIGMAR is its integration-stage estimate, and the two differ by up to 2.3x. The XDS cell now prints both as postrefined|MLE so a per-image estimate is compared against the one measured the same way. Fixes a scoping bug in the same addition where every crystal read the last directory's INTEGRATE.LP. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-04 10:52:30 +02:00
leonarski_fandClaude Opus 5	2c94f3013e	rugnux: fit the mosaicity from the strongest spots only ... The per-image mosaicity MLE ran over the whole indexed spot list, so it rode on --max-spots, which is an indexing budget. A spot is detected when I_full * R(tau) clears the finder threshold, so selecting by intensity censors on R(tau): a deeper list holds proportionally more large-|tau| partially recorded spots and the fit widens with it. Raising the budget 250 -> 1000 widened sigma_M 0.059 -> 0.075 deg on a rotation dataset whose measured rocking width says 0.054. That is not cosmetic. An over-wide mosaicity mis-states every partiality in scaling: forcing the mosaicity across that range moved the merge error model from b 0.039 / ISa 26 to b 0.167 / ISa 6, and the space-group search lost a genuine 422 with it, merging the crystal in 222 instead. Cap the fit at the strongest 250 spots. FilterSpotsByCount leaves the list strongest-first, so this selects exactly the spots a smaller --max-spots would, and the mosaicity becomes invariant: 0.0538 deg at 250, 500, 1000 and 2000 spots, with the correct space group at each. Trimming or down-weighting the tau tail does not work - the censoring is multiplicative in R(tau), so it widens the whole distribution rather than adding a tail. Battery over 37 crystals: exactly one change, the demoted crystal repaired (33 space groups matching XDS -> 34). 23 of 37 are bit-identical, never reaching 250 spots. Unaffected elsewhere: the default spot count is 250, and stills have no goniometer so they return before the fit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-04 07:34:40 +02:00
leonarski_fandClaude Opus 5	f25fea7024	rugnux: keep 1000 spots per image instead of 250 ... Offline reprocessing is not bound by the online spot budget, and the cap is applied at the end of SpotAnalyze, so it is exactly the spot list the indexer and the per-image refinement see. jfjoch_viewer already sends 1000, so the two front ends now agree on the same file. Measured as a paired A/B over the 37-crystal rotation battery, de novo, with the resolution and Friedel setting matched to the XDS reference, both arms from the same binary bar this constant: R_meas low shell 16 better 0 worse 19 unchanged R_meas 14 better 4 worse 17 unchanged ISa 14 better 6 worse 15 unchanged CC1/2 6 better 3 worse 26 unchanged Low-resolution R_meas is a clean sweep. Around half the battery is bit-identical: those frames never reach 250 spots, so the cap never bound. Wall clock is unchanged (10m00s vs 10m44s, uncontrolled for page cache). Known cost, and the reason this is its own commit: one crystal in the battery reproducibly loses symmetry, tetragonal 422 -> orthorhombic 222, doubling its asymmetric unit. Its R_meas and ISa "improve" there, but that is what merging in too low a symmetry always does, and the lower symmetry then admits a merohedral indexing ambiguity. An intermediate cap of 500 demotes it too, so it buys none of the safety. This is the known point-group-decision-moves-with-data-amount fragility of the space-group search rather than an argument for starving the indexer of spots - the search is the thing to fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 22:08:21 +02:00
leonarski_fandClaude Opus 5	2473e03cf7	rugnux: default --spot-sigma to 4.0, the value the viewer already uses ... The two front ends disagreed on the fixed-threshold spot finder: rugnux started from 3.0, jfjoch_viewer from the SpotFindingSettings default of 4.0, so the same file processed either way could give different spots. Inert on the default path - the adaptive finder derives its threshold from each image's own per-resolution-ring noise and never reads signal_to_noise_threshold (only ImageSpotFinderCPU/GPU and DetModuleSpotFinder do). It changes behaviour only under --no-adaptive-spots, and there it now matches the viewer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 22:07:58 +02:00
leonarski_fandClaude Opus 5	fbc5078399	compression: use the NEON bitshuffle on aarch64 ... bitshuffle_hperf is an x86-only implementation - its entire vector body sits behind __i386__/__x86_64__, so on aarch64 every entry point compiles down to the scalar fallback. Measured against its own SIMD path that costs 8.3x on encode and 3.8x on decode, and it is the transform behind every compressed image the writer produces and every one the reader, preview and XDS plugin take apart again. The classic bitshuffle vendored beside it does have an aarch64 NEON path, and is already compiled into the same target, so this costs nothing new. BitShuffleBlock.h picks bshuf_trans_bit_elem/bshuf_untrans_bit_elem there and keeps bitshuf_encode_block / bitshuf_decode_block everywhere else, where hperf is about twice classic SSE2 and remains the better choice. The expected aarch64 gain is ~2.5x encode / ~1.7x decode: classic NEON is 128-bit and carries an extra pass, so it recovers part of the gap rather than all of it. The condition mirrors USEARMNEON in bitshuffle_core.c exactly, because with NEON off the classic scalar path is slower than hperf's and must not be selected. Swapping implementations is only safe while the two agree bit for bit - otherwise an ARM build would write files an x86 build could not read. They do: verified byte-identical output and mutual cross-decoding for elem_size 1/2/4/8 over block sizes from 8 to 65536 elements. Both are always compiled in, so the new test holds them to it on every architecture, not just the one that would notice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 21:27:27 +02:00
leonarski_fandClaude Opus 5	cd0c43c65c	docs: add the packaging and documentation work to the rc.161 change list ... Covers the Debian package rename and the DKMS scripts, the writer's --verbose, the version plumbing, and the documentation pass - the release-contents page, the corrected repository URLs and package names, and the rugnux, viewer and writer pages. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:53:48 +02:00
leonarski_fandClaude Opus 5	cfcb84aece	docs: bring the tool pages back in line with the programs ... rugnux gained --model - R-free and 2Fo-Fc/Fo-Fc maps against an atomic model, and with it the resolution of the enantiomorph and of a merohedral indexing ambiguity - without the page ever mentioning it. It was the only option missing; the two lists now agree in both directions, checked against the usage the binary prints. The viewer page still said results are never saved and that no Windows package exists. Both have been false for a while: the Processing panel runs full rugnux jobs on the open dataset, writes _process.h5 and the merged reflections, registers each run as a selectable view so runs can be compared, and can hand out the equivalent command line for a cluster; and the installer is published with every release. The mask menu also loads TIFFs now, and the View menu has layout presets. The writer page documented -R for the root directory, which is the back-compatibility alias for -d, and an HTTP status interface that no longer exists - status reaches the broker over the writer notification socket, and a writer is stopped with a signal. The test page pointed at .gitlab-ci.yml and at jfjoch_offline_process, which is not a binary any more; the CrystFEL fixture pointed at HDF5DatasetWriteTest, which is not either. The broker page linked ../broker/redoc-static.html, which MyST resolved by copying the 700 kB file into _downloads/ rather than using the copy already in _static. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:53:37 +02:00
leonarski_fandClaude Opus 5	46930142dd	docs: write down what a release ships ... Nothing said what is in a release or what it needs of the machine it lands on: that the Linux binaries are built -march=x86-64-v3 and the Windows ones /arch:AVX, so each has a CPU floor; that the portable .tgz is built on RHEL 8 for its glibc; that the Windows installer is MSVC (Visual Studio 2026), CUDA 13.3, Qt 6.11 and carries the Qt runtime; and above all what the CUDA variants need. Only cuFFT is linked dynamically, and it has no link-time dependency on the driver library, so a CUDA build starts on a machine with no NVIDIA GPU at all and falls back to the CPU path - as long as cuFFT can be loaded, which the .tgz and the installer arrange by shipping it and the distribution packages arrange through the distribution's own CUDA packages. Collected into a new page rather than scattered over the install instructions. The repository page had the RHEL 9 rows pointing at el8 paths under the wrong slsdet number, no rows at all for the two slsdet9 repositories the pipeline uploads, a driver package named jfjoch-driver where it is jfjoch-driver-dkms, and a note that RPMs are unsigned from before the pipeline started uploading them with sign=true. The FPGA page had a paragraph that stopped mid-sentence, in the middle of a link, and a section describing a firmware build triggered by commit message. The firmware is stable and carried from version to version now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:53:19 +02:00
leonarski_fandClaude Opus 5	d615b900dd	ci: correct the tools the viewer archive is said to carry, and two dead conditions ... rugnux_scale and azint have not been separate binaries since they became rugnux --scale and rugnux --azint-only; the viewer component carries rugnux, jfjoch_extract_hkl and jfjoch_recompress. Two job conditions tested github.ref_type against 'workflow_dispatch'. ref_type is only ever 'branch' or 'tag', so that test never matched and never did anything: build-rpm's whole condition was that test, and half of the unit-test one was. Both jobs ran on a dispatch, as the release flow needs them to. Drop the dead tests rather than repair them - the behaviour they read as intending is not the behaviour that is wanted - and write down why unit-tests is skipped on a tag, which is the part that is deliberate: the dispatch run is the one that tests, and the tag it creates only rebuilds and uploads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:53:04 +02:00
leonarski_fandClaude Opus 5	0dd837e8e4	rugnux: name the third correction surface in the usage message ... --no-scaling-corrections said it disabled the decay and absorption surfaces. It disables the modulation surface too - GetCorrectionSurfaces() gates all three - and the flag has worked that way since the detector-plane modulation was added; only its description did not follow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:52:41 +02:00
leonarski_fandClaude Opus 5	6ae4cfc9e5	writer: take --verbose without an argument, and say what -d takes ... --verbose was declared required_argument while -v carries no colon in the getopt string, so the long form consumed whatever followed it. "jfjoch_writer --verbose tcp://host:5400" swallowed the address as the flag's argument and then failed for want of a data source - the short form was fine, which is presumably why it went unnoticed. The usage line for the root directory asked for <int>. It is a path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:52:41 +02:00
leonarski_fandClaude Opus 5	8712160adb	scripts: point the generated Python client at gitea, and make the scripts runnable ... The openapi-generator invocation still passed --git-host=git.psi.ch and a user id of jungfraujoch, from before the move to gitea.psi.ch/mx/jungfraujoch. Those properties are not cosmetic: they become the source URL in the generated README and pyproject, so the published client documentation - docs/python_client/README.md, which is copied out of the generated tree - told readers to pip install from a host that no longer answers. Regenerating with the corrected flags changes those two lines and nothing else, verified against the committed tree. update_version.sh, make_doc.sh and gen_python_client.sh were all mode 644, so the "run update_version.sh" the documentation asks for fails on the shebang. CMake and the CI both work around it by invoking them through bash. make_doc.sh builds a throw-away venv in the working tree and deletes it on the last line, which set -e skips whenever pip or sphinx fails - so a failed docs build left tmp_venv/ behind. Delete it from a trap instead, and ignore it along with the default output directory and the sdist directory gen_python_client.sh creates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:52:29 +02:00
leonarski_fandClaude Opus 5	22d75e2f01	build: take the project version from the VERSION file, and read it once ... PROJECT() carried a hardcoded 1.0.0 next to a JFJOCH_VERSION read from the VERSION file, so PROJECT_VERSION was free to drift from the version everything else uses. It cannot simply be handed the same string - project(VERSION) accepts numeric major.minor.patch only, and rejects a pre-release suffix such as -rc.161 - so cut the numeric part out of the same file instead of writing it down a second time. common/ then read ../VERSION again into PACKAGE_VERSION, purely to interpolate it into GitInfo.cpp. That is the same file read twice with two variable names, one of them a common enough name to be set by something else in the parent scope. Use JFJOCH_VERSION, which is already in scope there. The CUDA architecture note claimed the list "embeds no PTX". A bare entry in CMAKE_CUDA_ARCHITECTURES emits SASS and PTX both, so the newest entry has been the forward-compatibility path all along: on a GPU newer than anything listed, the driver JIT-compiles that PTX at first launch. Adding sm_121 still buys native code on Spark instead of a JIT, which is what the comment should have said. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:52:09 +02:00
leonarski_fandClaude Opus 5	0003acf2e4	packaging: name the Debian broker package jfjoch, and let its DKMS scripts run ... CPACK_DEBIAN_MAIN_COMPONENT does not exist. Only CPackRPM.cmake has a MAIN_COMPONENT variable, which is why the RPM came out as "jfjoch" while the .deb of the same component came out as "jfjoch-jfjoch" - the DEB generator names every component package "<CPACK_PACKAGE_NAME>-<component>" unless CPACK_DEBIAN_<COMPONENT>_PACKAGE_NAME overrides it, and the line we set was read by nobody. It also named a component ("broker") that does not exist, so it could not have matched anything either way. Set the name the generator actually reads, and declare Replaces/Conflicts on the old one: the new package owns the same files, so dpkg would otherwise refuse to unpack it over an installation that already has jfjoch-jfjoch. The DKMS component's postinst and prerm are the driver's postinstall.sh and preuninstall.sh, configure_file'd into place. They were mode 644, and CPACK_DEBIAN_PACKAGE_CONTROL_STRICT_PERMISSION is off, so they were packaged as they are - and dpkg cannot execute a maintainer script it cannot execute. The RPM path is unaffected: it reads the same files as text into the spec. The viewer's Freedesktop menu entry and its icon are installed on Linux only now. On Windows the Start Menu shortcut comes from CPACK_PACKAGE_EXECUTABLES and on macOS from the .app bundle, so on those two the installer was carrying a share/applications entry and a share/pixmaps icon that nothing reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:51:55 +02:00
leonarski_fandClaude Opus 5	ceb92fc4cc	docs: bring the rc.161 change list up to date ... Covers the GPU decode work (fused un-transpose, the memory it frees, corrupt-chunk detection, large bitshuffle blocks, host fallback), the two reproducibility fixes (integer ring statistics, striped geometry-refinement sampling), the connection-teardown and written-extent fixes, and the thread-count, max_hkl and compression-time repairs. Folded into the existing entries where they belong rather than added as new ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 16:58:03 +02:00
leonarski_fandClaude Opus 5	d79b20e268	indexing: key the shared device tables on their content, not only on an address ... The cache returned a device copy for a (device, host address) pair and cast it to whatever the caller asked for, with nothing checking that the bytes behind that address were still the same bytes. A host buffer can be mutated in place - PixelMask::LoadMask does exactly that - or freed and reallocated at the same address, and either hands the caller a device copy of something else. Nothing would report it: the tables are read-only geometry, so the engine would simply mask the wrong pixels for the rest of the run while the azimuthal mapping, the written pixel_mask dataset and the viewer overlay used the new one. Today that is unreachable, but only because of two guards in unrelated files that neither state nor assert the requirement. The byte length and an FNV-1a checksum of the bytes being uploaded are now part of the key. Both are computed once per engine construction, over a buffer that is about to be copied to the device anyway, so the cost does not show. Expired entries are pruned on insert, since distinct content now means distinct entries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 16:33:24 +02:00
leonarski_fandClaude Opus 5	6b713cf3da	rugnux: size the written datasets by the ordinals reached, not by the successes ... Each image is written at its own ordinal, so a frame that fails to load or analyse leaves a HOLE - the frames after it keep their positions rather than shifting up. The end message nevertheless reported the number of frames that SUCCEEDED as the image count, and the writer sizes /entry/data/data and every per-image array from that. So one failed frame in the middle of a run made the declared extent one short, and the image it dropped was the LAST one written, not the one that failed. Two failures dropped two, and so on: the file quietly ends before the data does, with the per-image metadata still carrying rows for images the VDS no longer maps. It also under-counted the data files when the run was split. Track the highest ordinal actually written and use that. A hole then reads as the fill value, which is what a frame that was never written should look like, and the counts of collected and written images stay counts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 16:33:24 +02:00
leonarski_fandClaude Opus 5	5727cb68a4	rotation_indexer: write down why the supercell bar is unreachable, and what failed to fix it ... `frac > RATIO * best_frac` cannot be satisfied once best_frac passes 1/RATIO - above 0.667 for a ratio of 1.5, which is ordinary for good rotation data. Above that the two guards do not raise the bar, they close the branch: no axis multiple and no lower-symmetry setting can displace the incumbent however much better it fits, so a genuine superstructure is kept as its sub-cell and its satellite rows go unindexed, silently. The obvious repair - restate the bar on the fraction left UNINDEXED, which is well defined over the whole range - was implemented and measured. It regressed the 37-crystal battery from 34/37 to 32/37 correct space groups: a C2 lattice fell to P1, and a P2 case went to C222 keeping 2923 of 22440 reflections with CC1/2 in the last shell at -35%. The indexed fraction is too noisy to carry a looser test. So the unreachable-but-safe form stays, and the limitation is recorded at the comparison rather than left to be rediscovered. Fixing it properly needs the selection to be decided on something better than the indexed fraction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 15:32:55 +02:00
leonarski_fandClaude Opus 5	abb94ca450	spot_finding: accumulate the adaptive ring statistics in integers ... The per-ring sums were floats reduced by atomics, so the ring sigma - and with it the detection threshold - depended on the order the blocks happened to arrive in. Detection compares an INTEGER pixel value against that threshold, so a threshold that drifts across an integer flips every pixel of that value in the ring at once, which is how a last-bit difference turned into a different spot list. A preprocessed pixel is an exact int32 and the masked and saturated sentinels are skipped, so v and v*v are exact in 64 bits, and integer addition is associative: the sums no longer care about arrival order. Both engines now accumulate the same way, so they agree exactly rather than approximately, and the GPU spot list is bit-identical across runs. The corrected sums that feed the reported azimuthal profile stay float - a pixel value times a float correction has no exact integer form - but they do not enter the detection decision. Cost: the ring reduction needs 28 bytes per bin instead of 20 in the plain pass, which drops it from eight co-resident blocks per SM to seven and costs about 11% of that kernel (0.582 -> 0.650 ms/frame on a 4.5 Mpx frame). End to end it does not show: alternating runs on three rotation crystals came out the same or slightly faster, and the battery is unchanged in every number. The CPU engine got 30% faster (32.2 -> 22.6 ms/frame), integers being cheaper than doubles. Tests: exact CPU/GPU agreement on the spot list, and 50 repeats of bit-identical output where there were four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 15:19:11 +02:00
leonarski_fandClaude Opus 5	212fbf9bab	rugnux: make the geometry-refinement sample deterministic, and spread it over the run ... The stills first pass drew frames from a shared cursor and stopped when a shared counter reached its target, which got two things wrong at once. The cursor walked the equally spaced sample in ascending order, so stopping early read only its leading PREFIX - the beam centre, distance and cell were fitted to the beginning of the run, not across it, and the comment claiming otherwise was wrong. And where the stop landed depended on how the workers happened to interleave, so the set of frames varied run to run: on the same data at -N 32 and -N 8 the pass examined 483 and 457 frames and refined the detector distance to 168.0481 and 168.0530 mm. The sample is now cut into a fixed number of interleaved stripes, each stopping once it has contributed its share. Every stripe spans the whole run, so an early stop no longer biases the fit, and a stripe is processed identically whichever worker claims it - so what gets examined depends only on the data, not on timing and not on -N. The same three runs now give 451 frames examined and 168.0452 mm, identically. The bundle selection was order-dependent too: frames are collected in worker-completion order and sorted by spot count with a non-stable sort, so equally strong frames swapped places between runs. They carry their image ordinal now and it breaks the tie. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 15:19:11 +02:00
leonarski_fandClaude Opus 5	5f8d37cdab	image_pusher: serialise tearing a connection down ... Two futures per connection are joined from more than one path: the acceptor reaping a dead connection, and the control plane starting or ending a run. Calling get() on one future from two threads at once is undefined and invalidates it, and the guard against it was a non-atomic test-then-set of an atomic flag - `if (!c.active) return; c.active = false;` - so both callers could pass it. Holding connections_mutex across the joins is what the previous commit removed on purpose, and rightly: the joins block on a writer that may be inside a send. So the lock is per connection and covers only the teardown. Nothing it joins takes it, so it cannot deadlock. The comment claiming a detached connection is unreachable by anyone else was wrong: a control-plane call that copied the pool before the erase still holds a shared_ptr to it. That is precisely how the two teardowns meet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:19:27 +02:00
leonarski_fandClaude Opus 5	2d3c39c9dd	image_preprocessing: write whole elements out of the un-transpose ... The raw-bytes path assembled each element a byte at a time, which on a full frame cost about 4x against writing the 8 contiguous elements a thread owns through an element-typed pointer. They are 8*ES-byte aligned, so the compiler merges them. 72.4 MB frame: 1.524 -> 0.406 ms for upload plus both kernels. The test now also times the LZ4 pass on its own, so the bounds and validity checks in the hot loop can be costed rather than guessed at. They are free: 0.231 ms against 0.2297 ms measured for the kernel before any of them existed - the restored offset == 1 and power-of-two fast paths pay for them. compute-sanitizer memcheck reports no error over 400 single-bit-corrupted payloads and nine malformed containers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:16:58 +02:00
leonarski_fandClaude Opus 5	b6c4a59d69	docs: record the stored-format break, and say how the adaptive finders actually accumulate ... The per-image image-scale B factor was dropped from the CBOR stream and from the written HDF5, which is a change for anything reading those files, but the changelog listed it only under the OpenAPI breaking changes. The GPU adaptive finder test claimed both finders sum the rings in double. The CPU one does; the GPU one stages a block's contribution in float before reducing across blocks in double, deliberately, to keep the hot loop's shared footprint down. Say so, and say what follows from it - detection compares integer pixel values, so a threshold that crosses an integer flips every pixel of that value in the ring at once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:14:24 +02:00
leonarski_fandClaude Opus 5	4b1c611bdf	image_analysis: query the current device, and upload the resolution mask on the engine stream ... Two leftovers from earlier fixes of the same shape. BraggIntegrationEngineGPU still read device 0's shared-memory size to decide whether its profile grid fits; workers are pinned round-robin across GPUs, so on a heterogeneous node that check can pass on a different card than the one the kernel launches on. SpotExtractorGPU still uploaded its default resolution mask with a pageable copy on the NULL stream, which is not ordered against the engine stream now that streams are created non-blocking. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:14:24 +02:00
leonarski_fandClaude Opus 5	32ecc296b8	broker: bootstrap a concrete max_hkl, as the API and the docs already claim ... How far the predictor walks the lattice became a setting defaulting to "derive it from the refined cell", which is what is wanted offline. Online it is not: per-image prediction cost then depends on whichever crystal is mounted, and the cube (2n+1)^3 grows quickly. The broker was only ever handed a concrete value when a client sent a bragg_integration block, and none of the shipped configs has one, so the derived path was the normal deployment. Four places in the tree - the conversion, the predictor, the OpenAPI description and the changelog - already state that the broker keeps a fixed bootstrap. Make that true. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:14:09 +02:00
leonarski_fandClaude Opus 5	42eb5bbe83	rugnux: clamp the refinement thread count to what the setting accepts ... RefineThreads rejects anything above 64, and the two-pass rotation path - the default - passed nthreads/2 straight from the machine. On a host with 130 or more hardware threads rugnux therefore exited 1 with the bare message "Candidate-cell refinement thread count" before reading a single image, and the only workaround was to discover -N and pass a smaller value. Dual 64-core parts are squarely in range. The limit is now a named constant the setter and the callers share, so the two cannot drift apart again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:14:09 +02:00
leonarski_fandClaude Opus 5	bec7e2e922	image_preprocessing: fuse the bitshuffle inverse with preprocessing, and verify the decode ... The device decoder was byte-exact on every valid input - 994 production-compressed images, 927 hand-built LZ4 blocks covering engineered (offset, matchlen) pairs across the overlap branch boundary, 18000 repeat decodes, sanitizer-clean - and an audit against LZ4_decompress_generic could not construct a valid block it mis-decodes. What it did not do was notice when the input was NOT valid, and that mattered more than it looks: the decode buffers are reused frame to frame, so a block that stopped early left the PREVIOUS image in place, and in the bitshuffled layout the untouched tail is the most significant byte-plane. A corrupt chunk therefore did not look like a missing corner. It looked like thousands of real pixels several powers of two too bright, fed to spot finding with no diagnostic, where the host decoder had raised an error. So the kernel now flags a block that fails to reach its declared length while consuming exactly its payload, and the host turns that into an exception once the caller has synchronised. Reads are clamped against the end of the payload as well as the output, both length chains are bounded exactly as read_variable_length bounds them, the two offset bytes are bounded, and LZ4's parsing restrictions are enforced. On the host side a block size that is not a multiple of 8 elements is rejected (it made the un-transpose read uninitialised shared memory), the block count is bounded by what the chunk could hold before it becomes an allocation (twelve header bytes could demand hundreds of MB of pinned memory, permanently, per worker), trailing bytes are rejected, and the stream is synchronised before any throw that happens after work is queued. An image of fewer than 8 elements is all verbatim tail and now decodes rather than throwing. When the device route fails for any reason the host decoder gets its turn, so it costs speed rather than the acquisition. The lanes cooperate on the copies and a later match can read bytes another lane wrote, which since Volta needs an explicit __syncwarp(); it worked only because ptxas happened to reconverge at the post-dominator. The prototype's offset == 1 and power-of-two fast paths are also restored - the shipped kernel ran a runtime modulo, an emulated 32-bit division per output byte, on the path its own comment calls the common case. The un-transpose is now fused with preprocessing. One thread owns one group of 8 elements across every byte-plane, so once it has transposed its 8 bytes out of each plane it holds 8 complete elements and emits 8 finished int32 pixels with the mask, the error marker, the saturation cap and the statistics applied. The decompressed image is never materialised: 0.623 -> 0.411 ms/frame at 18 Mpx, 0.523 -> 0.340 with 8 concurrent workers. Staging nothing in shared memory also drops the 48 kB ceiling, which had made any file whose bitshuffle blocks exceed it a hard failure; 64 kB blocks now decode. gpu_compressed is sized from the chunk with grow-on-demand instead of from the uncompressed size - it was reserving ~73 MB per worker to hold ~4 MB. Measured on a 1630x1553 uint32 rotation set at -N 32, peak GPU memory falls 3756 -> 3084 MiB; the same model gives ~144 MB per worker on an 18 Mpx frame. Decoding on the device also stopped reporting a decompression time, which blanked the broker's compression plot trace and filled /entry/profiling/compressionTime with NaN. The decoder brackets the decode with CUDA events and reports it again. Tests: a differential fuzz suite against the CPU decoder - incompressible and highly compressible data, engineered offsets, a size sweep hitting every rem%8 value twice, all six element sizes, an 18 Mpx frame, decoder reuse, concurrency, hand-built LZ4 blocks across the overlap boundary, 26 foreign bitshuffle block sizes from 128 B to 64 kB, corrupt payloads and malformed containers, with a coverage report that proves which LZ4 paths were reached rather than assuming it. Plus the fused path held byte for byte against ImagePreprocessorCPU, statistics included, and against the host-upload path on the same frame. Battery: 37 crystals, every merged number identical to the host-decode run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:13:55 +02:00
leonarski_fandClaude Opus 5	47277674fa	docs: split the breaking API changes out of the rc.161 change list ... Earlier releases put breaking changes in their own paragraph after the bullets (rc.139, rc.29) rather than as one item among them. Follow that: the OpenAPI changes now sit under their own heading below the list, with the client-side action in the lead line, and are listed one per change instead of run together. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 07:46:57 +02:00
leonarski_fandClaude Opus 5	737cbde3ff	docs: tighten the changelog and the decoding section for release ... The rc.161 changelog had grown entries several hundred words long and listed the same area three or four times over. Collapse them by subject - spot finding, resolution limits, space-group search, scaling, performance, correctness - and hold each to one line, keeping the actionable detail in the breaking API entry. Add the performance work that had not been written up: device-side image decoding and the parallel first-pass candidate-cell refinement. Section 0 of the CPU analysis document was the longest thing in it after two core algorithm sections, and most of that was a profiling narrative and the measurements that motivated the change rather than a description of what runs. Cut it to the two kernels, the host-side block scan and the fallback rule. The attribution stays; it is also in the reference list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 07:44:55 +02:00
leonarski_fandClaude Opus 5	7e47afe47f	rugnux: parallelise candidate-cell refinement, and stop repeating work in the tail ... Three independent changes to the CPU-bound parts of an offline rotation run, none of which alters a result. Candidate-cell refinement now splits across threads. RefineCandidateCells already took a (block, nblocks) partition, but the only call site passed nblocks=1, so the whole first pass of a two-pass rotation run sat on one thread per scheme - two threads, unchanged at every -N, for a third of the run. A block touches only its own scores(j) and cells rows and holds its own scratch, so the split is exact. The budget is a new IndexingSettings::RefineThreads, left at 1 by default and set only where few indexer threads exist: raising it unconditionally would oversubscribe the paths that already run one indexer per image across all workers. The mmCIF writer built a std::ostringstream per formatted number, twelve per reflection. snprintf gives the same digits for 0.535 -> 0.220 s per file. The space-group search built the same orbit mapping twice per candidate point group - once for the merge chi^2 and once for the systematic-error b, an apply_to_hkl and Canonicalize per observation per operator each time. Build it once and hand it to both. 18 Mpx rotation set 24.6 -> 18.7 s, 2.5 Mpx 13.0 -> 10.7 s, and the 37-crystal battery 13m55s -> 10m47s with no failures, the same 34/37 space groups, and statistics unchanged on 30 of 37 (the rest drift within the run-to-run spread the binary already had, which a control build with the split disabled reproduces). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 07:30:33 +02:00
leonarski_fandClaude Opus 5	13aa20a528	bragg_integration: grow the GPU reflection arrays with slack ... EnsureCapacity resized its 13 device arrays to exactly the current image's predicted-reflection count, so every image that set a new record freed and reallocated all of them. cudaMalloc and cudaFree take a device-wide lock in the CUDA driver, so those images stalled every other worker: sampling the worker threads during the per-image loop found 21-24 of 32 parked in cuMemAlloc_v2 or cuMemFree_v2, all called from this one function, and the running maximum makes 32 workers do far more allocator work than one does. Grow by half again instead. All transfers and kernel launches are sized by the per-image reflection count rather than by the capacity, and the member is already documented as holding at least that many, so over-allocating changes no result. On an 18 Mpx rotation set the integration stage drops from 1.37 to 1.25 ms per image at 32 workers; merged statistics, error model and adopted space group are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 01:02:07 +02:00
leonarski_fandClaude Opus 5	6e4c0ce202	image_preprocessing: decode bitshuffle+LZ4 on the GPU ... The pipeline decompressed each image on the host and uploaded the result. On an 18 Mpx rotation dataset that made the host-to-device copy the bottleneck of the whole per-image loop: nsys puts the copies at 78% of the loop against 39% for every kernel combined - 3600 transfers of 72.4 MB - and they ran at only 12.5 GB/s of an available 27-28 because the host-side decompression was itself saturating host memory bandwidth. The GPU was mostly waiting. So the compressed chunk goes across instead, about 4 MB rather than 72 MB, and is decoded on the device. That removes the transfer and the host decompression that was throttling it, in one change. Measured on an idle machine, a run goes from 45.11 s to 24.97 s - 1.81x - with the merged output unchanged. THE APPROACH IS JON WRIGHT'S (ESRF): "Experiences with GPU decompression for bitshuffle + LZ4 data", HDF5 User Group 2021, and github.com/jonwright/ bslz4decoders. The kernels here are ours, but the idea and the demonstration that it is worth doing are his. Cited in docs/ACKNOWLEDGEMENT.md and in the new section 0 of docs/CPU_DATA_ANALYSIS.md. Two kernels mirror the CPU decoder. LZ4 runs one WARP per bitshuffle block: every lane parses the same sequence stream (a broadcast read, no divergence) and the literal and match copies are split across the 32 lanes so the stores coalesce; an overlapping match is treated as a pattern of period offset sourced from bytes that already precede the write position, which keeps it parallel rather than a serial byte loop. One thread per block instead measured 13x slower. The bitshuffle inverse then un-transposes each byte-plane through shared memory and interleaves the planes back into elements. Only BSHUF_LZ4 is decoded on the device. The zstd variants have no device decoder, and neither has an uncompressed or float image; Supports() returns false for those and the caller decompresses on the host exactly as before. The fallback is explicit, so a format we cannot decode on the device is a slower path and never a wrong answer. Tests hold the device decoder against the CPU one byte for byte, on data from the production compressor, for every element size the detectors emit - including the 8-bit DECTRIS modes, which take bitshuf_decode_block's separate elem_size == 1 branch - plus a many-block frame, the formats it must decline, and malformed containers, which must throw rather than run off a buffer. Battery: 37 crystals, no failures, identical to the host-decode run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 00:16:36 +02:00