1182 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
leonarski_fandClaude Opus 5	9b55e44acd	Azimuthal integration: clamp a negative variance before the square root ... calc_std uses the cancellation-prone (sum2 - sum^2/n) form on float accumulators summed over millions of pixels, so a flat ring - true variance near zero - comes out negative as often as positive and GetStd() returns NaN. Both adaptive spot-finder ring accumulators already floor this at zero; this one did not, and 1a0774eed made sum2 correct, so the path is now actually exercised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:56:06 +02:00
leonarski_fandClaude Opus 5	2ef8841983	Azimuthal integration: keep the low Q limit below the maximum ... Making the high limit optional removed the implicit upper bound on the low one (it used to follow from high <= maxQ and high > low), so --azim-min-q 50 with no maximum is accepted and ResolveHighQ then calls std::clamp with its lower bound above its upper bound, which is undefined. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:56:06 +02:00
leonarski_fandClaude Opus 5	009555bc49	Spot finding: a zero high-resolution limit means no limit here too ... Every other reader of spot_finding.high_resolution_limit spells "unset" as value_or(0) and compares, so 0 and nullopt are interchangeable - except in SpotAnalyze, which passed the 0 straight to ResolutionShells and threw "Resolution must be above zero" on every image. Reachable over the REST API, where 0 is the natural way to say "no limit" and the settings check lets it through; the rugnux CLI already maps 0 to unset before this point. While here, check that a limit that IS set is finite regardless of its sign - NaN fails the > 0 test and was skipping validation entirely. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:55:19 +02:00
leonarski_fandClaude Opus 5	e123755c7d	rugnux: carry the point-group promotion into the second pass ... The two-pass rotation run reinstates pass-1's space group for pass 2's merge, so pass 2 skips the search block entirely - and with it the flag that records that the Laue class was CHOSEN by the search rather than given. The canonical output therefore printed the plain "no twinning: the Laue class is holohedral, so no merohedral twin law exists", which is exactly the circular conclusion the flag was added to replace; only the throwaway _01 output carried the caveat. The text is written per pass, inside RunPipeline, so the flag has to travel with prepass_merge_sg_ rather than being patched onto the returned result. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:54:53 +02:00
leonarski_fandClaude Opus 5	64c8117a96	Space-group search: absences always come from the all-observation merge ... The two-arm search is meant to use the Lorentz-filtered merge for the point group only - systematic absences live in the weak reflections a |zeta| cut removes, and reading them off the filtered arm is what cost four crystals their screw axes. That is what the code comment and CPU_DATA_ANALYSIS both say, but the filtered-arm-wins branch kept its whole result, screws and centering included. Let a search be pinned to a point group decided elsewhere (fixed_point_group) and re-run Stage B on the all-observation merge when the filtered arm rescues the point group. The point group is passed as its symmorphic representative, not by name: gemmi calls both P321 and P312 "32". Reporting that representative also lets the ambiguity check see two arms that disagree about which 2-folds are real - by name they looked identical - and the advice it prints now names a space group -S can actually be given. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:54:02 +02:00
leonarski_fandClaude Opus 5	1355d6b2aa	Space-group search: a screw threshold needs a row with controls behind it ... The row-relative cut scales the "too strong to be absent" threshold by the axial row's own median E^2, floored at the plain value - so it can only raise the bar, and a row whose control class holds a single strong reflection sets it from that one reflection. That direction invents screws: a genuine 4_2 whose 00l happen to be observed only at l=4n reads its l=4n+2 reflections as absent and ranks 4_1/4_3 above the truth. Require three controls before the row may set the scale; below that the row keeps the plain cut. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:52:32 +02:00
leonarski_fandClaude Opus 5	72cd91ccc3	Space-group search: judge a promotion against the worst of its tied parents ... A candidate can have several confirmed subgroups of the same order - 422 has both 4 and 222 - and on a twinned crystal the rival is not a harmless alternative: a P4 crystal twinned by 2[100] has 222 confirmed too, and 222 CONTAINS the twin laws, so its own merge b is already ballooned. The H test already answers to every tied parent; the systematic-b veto and rescue took whichever one the enumeration happened to list first (222 before 4, by space- group number), which disabled the veto on exactly the case it exists for. Take the smallest parent b, which is the conservative direction for both tests. The refusal message also quoted the raw parent b rather than the floored value the veto actually compared against. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:52:05 +02:00
leonarski_fandClaude Opus 5	609650b061	Rotation merging: keep the systematic sigma floor when ISa is unmeasurable ... The cap that refuses to report an impossible ISa was zeroing the asymptotic b itself, and that same value is the floor passed to SigmaWithSystematicFloor - where zero means "no floor". So on the degenerate low-multiplicity fit the guard is written for, instead of capping merged I/sigma at 100 it removed the cap entirely. Report the asymptote as unmeasured, keep the fitted value for the floor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:50:30 +02:00
leonarski_fandClaude Opus 5	386f10ad07	Merging: mirror the negative-intensity Poisson guard on the GPU ... 6be94f2be stopped subtracting a negative intensity's Poisson term from the background variance, but only in the host Combine(). The CUDA combine is the path that actually runs: Run() selects it whenever a device is present and no observation dump was asked for, so the correction never took effect on a normal run, and a --dump-observations run merged differently from a normal one - the two are meant to be identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:49:37 +02:00
leonarski_fandClaude Opus 5	a3feb1271c	docs: CPU_DATA_ANALYSIS describes the algorithms, not their history ... The document had accumulated development narrative: what was tried and rejected, which datasets a change rescued or cost, measured percentages from test batteries. That belongs in commit messages and reports, not in a reference description of the pipeline - it dates quickly, and a reader looking up what an algorithm does has to sort it out from how it came to be. Removed throughout, keeping the algorithmic content and the design reasoning that explains a choice on principle: * 3.2 the whole paragraph justifying the rotation spot-finding limit from battery measurements, and the CPU-vs-GPU per-frame timings; * 3.3 "a significance/z-score was considered but is uninformative"; * 7.4 / 7.5 the comparisons to a robust loss and to joint refinement as approaches that had failed; * 9.2 the R_meas / CC1/2 outcomes attributed to the trimmed-mean background; * 9.3 "per-detector-region and crystal-anisotropy profiles were evaluated and add nothing"; * 10.2 the stills tilt "succeeds where a freely-fitted width collapses"; * 10.5 the CC_anom argument, trimmed to why the statistic behaves as it does; * 10.6 the survey of per-frame correlation medians across datasets; * 13 the space-group bullet, restructured into the three gates it actually applies, dropping the dataset anecdotes; * 14.2 the free-form per-shell rescale, stated as a design choice rather than an experiment. Section 13's space-group text was one 20-line paragraph; it is now a numbered list of the three tests, which is what the code does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 10:11:33 +02:00
leonarski_fandClaude Opus 5	0f1851cf82	rugnux: drop the 1.5 A spot-finding limit on rotation data ... Rotation kept a 1.5 A high-resolution limit for spot finding on the strength of one indexing-rate measurement (100.0 -> 96.8% on a strong set). Measured properly, over the whole 33-crystal rotation battery, it does not earn its place: * no space-group decision changes - the same 30/33 agree with XDS, and the three that differ are the known pre-existing cases, unchanged; * 29 of 33 crystals are identical to the digit - same indexing rate, R_meas, CC1/2, ISa. The limit was doing nothing on the large majority; * where it does bite, the limit is the worse setting. The one crystal that loses appreciable indexing rate without it (99.50 -> 94.22%) comes back with lower R_meas (29.4 -> 28.1), higher high-resolution CC1/2 (27.9 -> 29.1) and higher ISa (5.77 -> 6.17). Another loses 0.4% of frames and gains 2.8 points of CC1/2_hi. Fewer frames indexed, better data from them; * runtime is unchanged (16m46s vs 17m32s over the battery). So the indexing-rate cost is real but does not carry through to the merged data, which is what the limit was protecting. Unset now means "as far as the detector reaches" for rotation as well as stills; --spot-high-resolution still sets a limit for weak, high-background data where the extra high-resolution spots are genuinely noise. This also removes the flag that distinguished "the user asked for no limit" from "the user said nothing" - with no rotation default left, both mean the same thing. While rewriting the comment block, corrects its neighbouring claim that rotation keeps the fixed-threshold finder; adaptive detection has been the default for both workflows since 6f4917dce. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 10:39:23 +02:00
leonarski_fandClaude Opus 5	323c76abde	docs: correct CPU_DATA_ANALYSIS against the code it describes ... Ten places where the document and the implementation had drifted apart. Each was checked against the source before rewriting: * 7.5 rotation post-refinement: it is TWO separate cross-validated steps (cell+axis from the angles, then distance+beam from the positions with the cell fixed), not one joint fit against the merged fulls; the held-out split is an hkl hash, not a frame split; the bounds are +-5% on distance and +-15 px on the beam, not "under ~1%"; and only the distance and beam centre reach pass 2, which re-indexes de novo. * 9.2 the trimmed-mean background: it is computed in the shared background pass, so it DOES apply to --integrator boxsum. Only the broadband sigma-clip is excluded. The section previously said both, contradicting itself two paragraphs apart. * 9.3 per-reflection profile rebuild, sub-pixel centring and radial elongation are gaussian-only; the empirical profile keeps the fixed per-shell grid and is accumulated on rounded predicted positions, not centroids. * 10.5 the asymptotic ISa and the b_ISa sigma floor are rotation-only; stills report 1/b and floor with the whole-range b. * 8.4 centering absences are applied only when the user fixes the space group - de novo, prediction runs in P so the search can confirm the centering from the intensities. * 10.6 per-batch relative-B cross-validates on ASU-group parity, not the frame parity the other surfaces use. * 13 the resolution cutoff sits one reported-shell width PAST the CC1/2 = 0.30 crossing, so data below 0.30 are kept. * 7.1 the orientation-only prior penalises all three components of the angle-axis vector. * 10.1 rlp is the RECIPROCAL Lorentz factor, L = 1/rlp. * 14.3 the model scaling is fitted over work and free reflections alike, so R-free is free of refinement, not of the scaling fit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 09:41:32 +02:00
leonarski_fandClaude Opus 5	648e8c62c8	CHANGELOG: record the two settings fields that are no longer required ... Making azim_int_settings.high_q_recipA and spot_finding_settings.high_resolution_limit optional changes the wire format: when unset they are omitted rather than sent with a placeholder, and a client generated from an older spec does j.at() on them. The azimuthal-integration limit now defaults to unset, so a stock broker omits it out of the box - the break needs no operator action to hit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 09:38:38 +02:00
leonarski_fandClaude Opus 5	c10d7ced17	rugnux: build the first-pass analysis engine once, not per image ... The first-pass spot cache constructed an MXAnalysisWithoutFPGA and an AzimuthalIntegrationProfile inside the per-image lambda, so every cache miss allocated a CUDA stream, the preprocessing buffer, the spot finder, the azimuthal integrator and the Bragg engine, used them for one frame, and freed them again - hundreds of times, serially, on the --redo-rotation-spots path. Both worker loops already hoist the same object out of their loop; only this path did not. Build them once for the whole first pass, and only when spots actually have to be found (with --reuse-rotation-spots there is nothing to allocate). Reuse is safe because every azimuthal-integration path - CPU, GPU and the fused adaptive engine - clears the caller's profile before adding to it, so each frame's output is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 09:38:11 +02:00
leonarski_fandClaude Opus 5	6be94f2be0	Merging: do not subtract a negative intensity's Poisson term ... The expected-variance weights decompose an observation's sigma^2 into a background part and a Poisson signal part, then rebuild the signal part at the reflection's merged mean. The decomposition subtracted corr*I with I taken as-is, so a negative I ADDED to the background part: an observation at I = -1.5 with sigma^2 = 1 came out with a base variance of 2.7 rather than 1. That inflates the variance of precisely the down-fluctuated observations the correction exists for. Below about one photon they are then under-weighted and the merged mean is biased high - the same direction of error, in the same regime, that weighting by the observation's own sigma produces. Subtract max(0, I) instead: a negative intensity has no Poisson signal to remove. Both users of the decomposition are fixed - the stills merge, where expected-variance weighting is now the default, and the rotation combine it was mirrored from, which had it first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 09:36:40 +02:00
leonarski_fandClaude Opus 5	59977ae910	rugnux: --spot-high-resolution 0 really means no limit on rotation data ... Passing 0 reset the limit to "unset" and logged "No high resolution limit for spot finding: as far as the detector reaches" - and then, 700 lines later, the rotation default put 1.5 A back, because unset carried two different requests: the user said nothing, or the user asked for none. The log said one thing and detection did another, and there was no way to lift the limit on rotation data at all. Remember whether the option was given, and apply the rotation default only when it was not. Documented in the usage message. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 09:35:36 +02:00
leonarski_fandClaude Opus 5	c40e09d458	rugnux: report an ambiguous space group instead of silently picking one ... The two-arm check for "same order, different symmetry" ran after the all-observation arm had already been adopted by value, so it compared that arm against a copy of itself: the point-group names were always equal and the branch was dead. In the other direction the orders were always unequal, so it was dead there too. The AMBIGUOUS warning - written for the case where the two merges support different symmetries of the same order, which implies two different molecular-replacement searches - could never be emitted. Check before the adoption, while both arms still hold their own result. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 09:34:41 +02:00
leonarski_fandClaude Opus 5	157698d31e	Space-group search: report the order of the point group it chose ... The two-arm search compared its arms by the order of the space group each had picked, but Stage B leaves best_space_group unset whenever no candidate is eligible - no candidate had enough observed absences to trust. That is not rare on the Lorentz-filtered arm, and for a systematic reason: the filter removes the badly-measured observations, which is where the weak systematically-absent reflections are. An arm that confirmed 422 but stopped short of naming a space group therefore scored order 0 and lost to an arm supporting P2, and the demotion was logged as "taking the higher symmetry" - the comparison and the message both wrong, in the one direction the design says cannot happen. Carry the point-group order in the result, set from the order Stage A actually adopted, and compare on that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 09:33:29 +02:00
leonarski_fandClaude Opus 5	0162afd324	Azimuthal integration: report the dimensions of the image the mapping covers ... The mapping took its width and height from the CONVERTED geometry unconditionally, while pixel_to_bin is sized per mode: converted when the geometry is transformed, raw module layout when it is not. In raw mode the two disagreed - 2068x2162 reported against a 1024x4096 map on a JF4M. Only the adaptive spot finders read those dimensions, and they read them for exactly the thing that breaks: the CPU finder derives npix = w*h and then indexes the image, pixel_to_bin and the resolution mask with it, so it walked ~277k pixels past the end of all three; the GPU finder stays in bounds but decodes the strong-pixel bit index with the wrong row stride and reports spots at wrong coordinates. Nothing combines raw geometry with adaptive detection today, so this was latent rather than live. Take them from GetXPixelsNum()/GetYPixelsNum(), which already follow the geometry mode. The converted path is unchanged - it is the same number there - and every internal use is inside SetupConvGeom, which only runs when the geometry is transformed. Covered by two tests: the mapping's dimensions must match pixel_to_bin in both modes, and the CPU adaptive finder must return a spot planted on a raw-geometry image at that raw pixel. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 09:31:58 +02:00
leonarski_fandClaude Opus 5	1a0774eed0	Azimuthal integration: clear sum2 along with sum and count ... Clear() reallocated sum and count but left sum2 as the constructor had sized it. Every caller reuses one profile across the images of a dataset (rugnux keeps one per worker, the viewer one per view), so the squares kept accumulating while the means restarted at zero: the per-image standard deviation written to /entry/azint and shown in the plots was meaningless from the second image on, and grew without bound over a run. The size mismatch was the sharper edge. Clearing to a mapping with a different bin count left sum2 shorter than sum, and GetStd() and operator+= then read past its end - reachable in the viewer by opening a dataset with a wider q range than the one before it. Both are covered by tests: the same frame twice with a Clear() in between has to give the same standard deviation, and a profile cleared to a wider mapping has to report the right value in a bin that only the wider mapping has. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 09:28:38 +02:00
leonarski_fandClaude Opus 5	8cb1cacadf	rugnux: show the space-group search on rotation data ... The search runs in pass 1 of the rotation two-pass; pass 2 only reuses the group it decided, so the result the CLI renders carried no search at all and the whole report - operator correlations, the space-group candidate table, the refused higher symmetry - was silently dropped on every rotation dataset. Most costly of all, the "or <group> (indistinguishable from these data)" line never appeared, so an enantiomorphic pair the intensities genuinely cannot separate was reported as a single answer. Carry pass 1's search into the returned result. Also de-duplicate the alternatives when the centred-lattice test swaps the metric-matching candidate into the answer: the group it displaced was left out and the chosen one listed twice ("C2 or P21 or C2"). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 08:27:43 +02:00
leonarski_fandClaude Opus 5	d28db19ab1	Space-group search: judge a screw axis against its own axial row ... A reflection the group predicts absent counted as a violation when I/sigma > 3 AND E^2 = I/<I>(shell) > 0.3. Neither half survives contact with real data: * merged sigma is floored at b|I|, so merged I/sigma saturates at ISa for nearly every reflection - the I/sigma half is an on/off switch keyed on ISa vs 3, not a per-reflection test. On one crystal the absent class read <I/s> 4.10 against 3.73 for the present class while being genuinely extinct; * <I>(shell) decays with resolution while a systematically-absent reflection keeps a small NON-decaying residual (background / profile leakage), so absent reflections drift over an absolute E^2 cut at high resolution. That cost a tetragonal 42_12 crystal its 4_1: 18 of its 47 absent 00l crossed the cut, all beyond 3.7 A, at absolute intensities identical to the low-resolution ones correctly judged absent, while their l=4n row-mates sat 20-60x higher at the same resolution. A screw extinguishes only the reflections that lie ON its axis, so the fair yardstick is the rest of that same row. The threshold is now 0.3 * max(1, median E^2 of the reflection's own row), the row being the gcd-reduced reciprocal-space direction and the control class the same-row reflections the group predicts present. Floored at 1, so it only ever relaxes: a screw can be recovered by it, never lost. Per row, not pooled. A 4_1 along c and a 2_1 along a are separate conditions with separate controls; pooling let the weak a/b rows (median E^2 ~0.5) set the threshold for a strong c row (8.4) and the rescue never fired. The candidate table now reports the screw evidence (median E^2 of the absent class and of its rows) - the <I/s> columns are the centering evidence and say nothing about screws, for the sigma-floor reason above. Rotation battery, 33 crystals: 31 decisions bit-identical, the 42_12 crystal recovers its 4_1 (0 violations, row E^2 8.4 vs absent 0.12), and one crystal with a long axis and heavy 00l overlap moves to a 4_1 group at exactly 10.0% violations - marginal, and its sister crystal of the same form sits at 13.3% and does not move. Real screws now span 0-9.3% violations, so max_absent_violation_fraction cannot be tightened below 0.10 without risking a genuine one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-29 08:27:34 +02:00
leonarski_f	43e9de9573	VERSION: 1.0.0-rc.161	2026-07-28 21:58:00 +02:00
leonarski_fandClaude Opus 5	40acf1ffc4	rugnux: search the space group twice by default on rotation data ... --search-min-zeta now defaults to 0.85 for rotation, so the de-novo search runs on a merge of all the observations AND on a merge of only the well-measured ones, and keeps whichever found more symmetry. Previously it shipped off and the second opinion had to be asked for. Rotation battery, 33 crystals, NO flags beyond the resolution limit: fixed-threshold finder 30/33 - zero crystals differ from the single search adaptive finder 30/33 - the same three mismatches Both arms now agree crystal for crystal, which they have not done before. The last disagreement was a thaumatin set whose 4-fold sits 88.9 deg from the spindle: at defaults it now reads P42(1)2 (all-observation merge -> 222, Lorentz-filtered -> 422, higher taken) where it read P222. The classic arm is a strict no-op - zero differences against both the explicitly-flagged run and the run predating the dual search - so the default costs nothing where the geometry is not the problem, and 47.2 s against 47.8 s on the same crystal back to back. The default is safe to set because the two searches can only disagree by a LOST operator: discarding observations starves an operator correlation, it cannot invent one. That also makes the 0.85 itself uncritical - too aggressive a cut only means the second opinion contributes nothing and the full merge wins. --search-min-zeta 0 restores the single search. Docs: CHANGELOG gains a 1.0.0-rc.161 section covering the branch, and CPU_DATA_ANALYSIS records the four analysis changes of this work - the confidence-weighted per-image refinement, the collapsed per-frame scale guard, the opt-in per-image rejection, and the operator-disagreement criterion with the two-search rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 21:56:36 +02:00
leonarski_fandClaude Opus 5	25458265d3	Space-group search: ask twice - all observations, and only the well-measured ones ... --search-min-zeta rescues a point group that the full merge cannot confirm, but used on its own it is a trade: on the crystal it was built for it recovers the correct 422, and on four others it costs the space group outright, because discarding 40-80% of the observations starves operator correlations that were perfectly healthy. Both ways of applying it - filtering the pairs that enter the statistic, and filtering the observations that enter the merge - trade the SAME crystals, so the cut itself is the problem, not where it is applied. Filip's observation makes it one-way: every disagreement between the two is a LOST operator, never an invented one. Discarding observations can starve a correlation; it cannot manufacture symmetry that is not there. So run the search on both merges and keep whichever found MORE symmetry, and the failure mode disappears - each arm rescues the other exactly where it fails. crystal all observations Lorentz-filtered adopted thaumatin (weak) 222 422 422 tetragonal lysozyme 422 222 422 cubic insulin x3 23 2 / 222 23 The filtered merge is used ONLY to rescue the point group. The screw and centering determination always comes from the merge with all the observations, because systematic absences are decided by the WEAK reflections and the filter throws most of them away. Preferring the filtered arm on a tie is not a conservative choice, it is a wrong one: it cost four crystals their screw axes (P2(1) read as P2, P4(1)2(1)2 as P42(1)2) with the point group and every intensity statistic identical - a regression invisible to CC1/2, R_meas and ISa. Where the two find the same ORDER but different symmetry, nothing can prefer one, so the run says so: it names both space groups, states that the data do not decide, reports which one processing continued in, and gives the flag to force the other. Two candidates of the same order imply different molecular replacement searches, and trying both is cheap next to reprocessing - much cheaper than a confident wrong answer. Rotation battery, 33 crystals, both spot finders: fixed-threshold finder 30/33 - ZERO crystals differ from the single search adaptive finder 30/33 - the same three mismatches, gap CLOSED The adaptive finder now matches the fixed-threshold one exactly, which it has not done before: its last remaining loss was the thaumatin set whose 4-fold sits 88.9 deg from the spindle, and it now reads P42(1)2 (all-observation merge -> 222, Lorentz-filtered -> 422, higher taken). A merohedral twin stays refused in BOTH arms at all three frame ranges where it over-promotes, and at one of them the second opinion is strictly better than shipping behaviour - the full merge collapses to P1 where the filtered one finds the correct H3. Cost is the extra scale-combine-merge on already-ingested partials, with no re-integration: 47.2 s against 47.8 s on the same crystal back to back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 21:11:05 +02:00
leonarski_fandClaude Opus 5	f2b92e3f4d	rugnux: --search-min-zeta drops badly-measured observations from the symmetry search ... zeta is the sine of the angle between a reflection's rocking path and the spindle. Near 0 the reflection crosses the Ewald sphere almost tangentially, spends many frames in diffracting position and is measured worst. The de-novo space-group search asks how EQUAL an operator's paired intensities are, so its answer is dominated by whichever reflections are measured worst - and when the spindle lies in a lattice plane, an operator that permutes the two in-plane axes samples a different mixture of measurement qualities than one that only flips signs. That is not a fair comparison, and it can make a real symmetry operator look like a twin law. Measured on a thaumatin set mounted that way (its 4-fold is 88.9 deg from the spindle), the added operators' disagreement is 1.74x the parent's over pairs where both reflections have zeta < 0.85 and 1.003x - i.e. the symmetry is exact - over pairs where both are above it. The search consequently refuses the 422 promotion and merges the crystal in P222, while the same data forced to the right group give CC1/2 99.2% at multiplicity 10.7, matching XDS. With the option the de-novo pass ignores those observations (the final merge keeps everything - there completeness is the point): zeta cut observations ignored H ratio adopted 0 (off) - 1.47 P222 0.5 1620648 1.44 P222 0.7 3006013 1.34 P21212 0.85 4536724 promoted P4212 (correct point group) OFF BY DEFAULT, and it must stay off, because the same cut costs four other crystals their space group (P41212 -> P212121, I23 -> P2, I23 -> I222 twice): at 0.85 it discards 40-80% of all observations, which on a crystal whose geometry is not the problem simply starves the search. Two independent implementations - filtering the pairs that enter the statistic, and filtering the observations that enter the merge - trade exactly the same crystals, so this is a property of the cut and not of where it is applied. Verified bit-identical to the previous binary when off. The companion diagnostic is already there: the run now reports how close a symmetry axis lies to the spindle, which is the geometry that makes this option worth reaching for. Implementation note for anyone tempted by the cheaper route: excluding these observations from the ASU grouping alone does NOT work. The 3D combine selects partials on corr, not on their group, so their intensity still reaches the fulls and the merged intensities are unchanged - measured, the statistic did not move by 0.03 while 67% of observations were nominally excluded. Zeroing corr is what removes an observation from the combine, the merge and the error model alike. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 20:03:43 +02:00
leonarski_fandClaude Opus 5	eb70684fa9	rugnux: report how close a symmetry axis lies to the spindle ... A rotation sweep never records the reflections whose reciprocal vector lies within the Bragg angle of the spindle - the blind cusp. Symmetry normally supplies them from an equivalent elsewhere in reciprocal space, so the hole closes. It cannot when a symmetry axis IS the spindle: the cusp is then mapped onto itself, every reflection in it is equivalent only to other reflections in it, and it stays empty however long the sweep runs. The user can fix this at the microscope - re-mount, or add a sweep on another axis - but only if they are told, and nothing in the output mentioned it. Report the smallest angle between any proper rotation axis of the adopted space group and the goniometer axis, always on rotation data, and warn when it falls under 15 deg. The axis is found by projecting onto each operator's invariant direction (the sum of its powers annihilates everything else) and mapping that fractional direction through the refined lattice into the lab frame; the angle is invariant under the sweep, so the reference orientation is enough. Cross-check: this reports 30.6 deg for a crystal whose 4-fold an independent analysis of the XDS orientation matrix put at 30.5 deg. Measured on three rotation sets: 13.6 deg (2-fold, warns), 16.2 deg (2-fold, 99.7% complete) and 30.6 deg (4-fold). The 15 deg bound is practical rather than derived - the blind cone's half-angle is the maximum Bragg angle, ~15 deg for 2 A data at 1 A wavelength - and the wording says what the diagnostic can honestly support: the angle is a risk indicator, the loss is confined to the cone rather than spread over the data, and overall completeness may still look reasonable while the region near the spindle is empty. It does not promise a completeness number, because across those three sets the overall figure does not track the angle (99.7% at 16.2 deg, 92.6% at 30.6 deg). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 17:59:47 +02:00
leonarski_fandClaude Opus 5	b81c6f00b7	reader: take the stored image bit depth from the file, not the DECTRIS default ... DetectorSetup hardcodes bit_depth_image = 16 for every DECTRIS detector (DetectorSetup.cpp:78), and GetByteDepthImage() consults that BEFORE the depth the reader takes from the file - so a file storing 32-bit images had its overflow computed as a 16-bit one. With the reader also declaring the images signed, GetOverflow() returned INT16_MAX and GetSaturationLimit() became min(file value, 32767). Every count above 32767 was therefore marked saturated, and because the integration accept gate requires ALL inner pixels valid, the whole reflection was discarded. That silently removes the strongest reflections of a strong crystal - the low-resolution ones that anchor scaling - while the file itself declares saturation at 105000-133000. Measured on a lysozyme rotation set (200 frames), before -> after: saturated pixels per frame 0.815 -> 0.000 brightest accepted pixel 32738 -> 87633 mean per-frame maximum 26218 -> 37390 i.e. the ceiling was exactly INT16_MAX and nothing genuine reached it. Which datasets this touches depends on how bright they are: measured pixels above the old ceiling range from 0.0 per frame on some rotation sets to 6.1 on others, so the fix is a no-op on weak data and only ever adds reflections. Rotation battery, 33 crystals: no point group changed (30/33 before and after) and no run failed. Four crystals move on quality, in both directions - ISa 1.90 -> 2.40 and 3.29 -> 4.80 on two, 2.97 -> 1.85 and 20.83 -> 18.47 on two others; three of the four are the battery's known weak or run-to-run-unstable crystals. The one strong crystal that moves gains 136 observations out of 1.9 million and loses 2.4 ISa: the reflections restored are by construction the brightest ones, and they carry the systematic error that the strongest reflections always carry. That is a real cost, but it is the cost of MEASURING them rather than discarding them unseen, and a lower asymptotic I/sigma on data that are now complete is preferable to a flattering one on data that quietly are not. Only the offline file reader is affected; the online path builds its detector setup from configuration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 17:49:14 +02:00
leonarski_fandClaude Opus 5	6f4917dcee	rugnux: adaptive spot detection is the default for rotation data too ... It was held back because a 33-crystal rotation battery showed it breaking three crystals deterministically - a lost space group, a halved indexing rate and a collapsed merge. None of those causes turned out to be in detection. The extra spots adaptive finds are real. Measured per spot against a finder-neutral local background: 64% recur at the same position on the adjacent frame (chance rate 0.5%) with 2-frame rocking curves, and 0.00% would fail a conventional local SNR >= 4 test, median local SNR 34. What they include is genuine peaks belonging to no lattice the indexer found, and the damage they did scaled with their absolute COUNT (80.6 per frame against 36.8 for the fixed finder), not with their quality - which is why nothing aimed at judging individual spots ever worked. The three failures fell to fixes elsewhere: merge collapsed - a per-frame scale free to collapse toward zero amplified two junk frames by 546x (704098712, ec7a82613). Not a detection problem at all: the fixed-threshold finder trips the same bug on a different frame range. space group lost - the per-image geometry refinement was dragged 2.3-2.8 deg off by the weak-spot tail in an unweighted fit; weighting each spot by how strong it is FOR ITS RESOLUTION fixed it (ae126c3d5), and gained a point group for the fixed finder too. indexing halved - gone with the same two; that crystal is now better under adaptive (CC1/2 92.6 -> 95.9, high-shell 44.9 -> 56.4). Battery, 33 crystals, adaptive vs the fixed finder: exact space group matching XDS 26/33 vs 25/33 point group matching XDS 29/33 vs 30/33 ISa better on 6 crystals recovers a screw axis the other misses (P321 -> P3121) The one point group it loses is a tetragonal crystal where adaptive collects 2.4x the observations at better R_meas (21.4% vs 28.3%) and better ISa (4.58 vs 2.91), and forced to the right group gives CC1/2 99.2% at multiplicity 10.7 - matching XDS. Only the automatic symmetry call fails there, and six candidate causes have been measured and refuted (mixed indexing hands, off-crystal frames, a badly integrated minority, radiation damage, pseudo-tetragonality, uncorrected anisotropy). It is left as the subgroup, which is the recoverable direction: -S gives XDS-quality data from the same run, whereas the failures this unblocks were not recoverable. --no-adaptive-spots reverts to the fixed-threshold finder. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 11:59:37 +02:00
leonarski_fandClaude Opus 5	4895dc1018	Rotation: let --min-image-cc drop frames that disagree with the merged reference ... The flag was accepted on rotation data and did nothing - it is read only by the stills merge (Merge.cpp), and the CLI warned about that rather than fixing it. Meanwhile RotationScaleMerge already COMPUTES a per-frame correlation against the merged reference and writes it to the per-image table; nothing acted on it. Wire the two together. A rejected frame has its partials' corr set to 0, which is how a frame already leaves the pipeline - every consumer requires corr > 0, so the combine, the merge and the error model all drop it together. The GPU path reuses the SmoothCorr kernel with a ratio of 0, so one implementation covers both. Off by default (0), and verified bit-identical to the previous binary when off. What it catches, on the two rotation datasets that have a population to catch: a two-lattice crystal - two lattices in two physical AREAS of the sample, so the sweep passes from one to the other and whole blocks of frames measure a different crystal from the one being merged (frames 500-700 index perfectly well at a per-frame CC of 0.22 against 0.47-0.56 either side, in 11 contiguous runs). R_meas 28.6 -> 24.6%, CC1/2 93.6 -> 95.1, high-shell CC 23.4 -> 38.3. a second dataset with 9.5% of frames below CC 0.30: R_meas 24.3 -> 23.4%, CC1/2 92.6 -> 93.4. The criterion is "this frame disagrees with the merged reference", NOT "this frame is off-crystal". It happens to catch both, because a frame that measures nothing and a frame that measures a DIFFERENT crystal fail the same test, and it does not need to know which. For the two-area case that is a workaround, not a treatment: it recovers one crystal by discarding the other, where processing the two as separate sweeps would keep both. The frame-block structure is clean enough that such a split could be detected automatically. WHY THERE IS NO DEFAULT. The per-frame CC is not comparable between datasets - it is as much a measure of data quality as of frame validity. Measured medians across the battery run from 0.30 to 0.81, so one absolute bound removes 13 frames from one dataset and 584 of 1800 from another: battery at --min-image-cc 30, 33 crystals: no point group changed (30/33), four crystals clearly better (one +5.4 CC1/2 points, the two-lattice case above, and ISa gains of 1.3-4.6 on three others) - and one healthy crystal lost a third of its frames and with them its high-resolution shell (CC1/2_hi 26.2 -> 2.0). This is the same trap as an absolute bound on any per-operator or per-frame agreement statistic, and the same one the per-frame scale guard avoids by measuring against the run's own median. A principled version would cut on the SHAPE of the per-frame CC distribution - a dataset with a bad subpopulation is bimodal, a uniformly weak one is not - rather than on an absolute value. Until that exists this stays opt-in, and the per-image CC it keys on is already in the _image.dat table for anyone choosing a value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 11:45:49 +02:00
leonarski_fandClaude Opus 5	11c7cab2e5	Space-group search: take the operator disagreement as a median, not a mean ... A merohedral twin mixes EVERY reflection with its twin mate, so it shifts the whole distribution of |I1-I2|/(I1+I2). A minority of badly measured reflections shifts only the tail. The mean cannot tell those apart; the median is blind to the second and just as sensitive to the first. Measured on real crystals, moving the statistic from the mean to the median leaves genuine promotions where they are and pushes every twin up: genuine tetragonal 1.016 -> 1.013 genuine lysozyme 1.051 -> 1.067 genuine tetragonal 1.238 -> 1.231 twin (-e 1050) 1.272 -> 1.447 twin (-e 450) 1.280 -> 1.622 twin (full) 1.441 -> 1.522 twin (-e 600) 1.427 -> 2.010 The margin around the 1.25 bound widens from 2.7% (genuine 1.238 against twin 1.272 - uncomfortably tight for a decision that cannot be undone downstream) to 17.5% (1.231 against 1.447). The bound itself does not move. Rotation battery, 33 crystals in both detection modes: no point group changed in either (30/33 and 29/33, as before), and only one crystal's numbers move at all - the one already documented as nondeterministic between repeat runs of the same binary. The synthetic twin-fraction x multiplicity grid passes unchanged. So this buys margin, not outcomes. Found while testing a different hypothesis, which the same measurement refuted: a tetragonal crystal whose 422 promotion is wrongly refused reads 1.484 by the mean and 1.472 by the median, i.e. its disagreement is distribution-wide and is NOT a badly-integrated minority. That crystal's cause is elsewhere and is not addressed here - see the note below. Its indexing-ambiguity operator (-k,-h,-l) lies INSIDE 422 but OUTSIDE 222, so the subgroup merge the search is given mixes lattices indexed in the two alternative hands. That corrupts exactly the 4-fold relationships and leaves the 2-fold ones intact - measured, the 222 step reads 0.917 and the 422 step 1.484 - and the corruption is indistinguishable from a twin law. Forcing the tetragonal group merges the two hands as equivalent and the same data give CC1/2 99.2% at multiplicity 10.7, matching XDS. The failure is worse the BETTER the frames index (99.9% vs 63.3% for the run that gets it right), because indexing more frames picks up more of both hands. So no statistic computed on a subgroup merge can arbitrate a promotion whose added operators include an indexing-ambiguity operator. Fixing that means resolving the ambiguity before the search, or detecting the coincidence and deciding another way; the operators needed to detect it are already computed (the run warns about them). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 11:07:06 +02:00
leonarski_fandClaude Opus 5	ae126c3d5b	Per-image refinement: weight each spot by how strong it is for its resolution ... `RefineGeometryIfNeeded` hands XtalOptimizer the WHOLE spot list, not the indexed subset, and the first pass admits anything within 0.3 fractional-Miller units of an integer - which is 11.3% of RANDOMLY placed spots, since the admitted volume is (4/3)*pi*t^3. Every one of them then enters an unweighted L2 fit with an arbitrary rounded index. On images with many detections the refined orientation ends up 2.3-2.8 degrees from the goniometer-consistent one and explains 14 of its own 250 spots where the undragged orientation explains 68; mosaicity and profile radius inherit the error and integration follows. Weight every spot by its intensity divided by the median intensity of its own equal-count resolution shell, applied as w^2 on the squared residual with w^2 = r/(1+r). The shell normalisation is the point: refinement needs the high-resolution spots because they carry the cell and distance, and those are LEGITIMATELY weaker, so a raw intensity weight would suppress exactly the spots the fit depends on. Measured, the weight is resolution-neutral - median exactly 0.707 in every shell, and corr(w, 1/d^2) = -0.20 / -0.11 against -0.32 / -0.34 for the same function of un-normalised intensity. This is a PRIOR: it is computed from the spot alone and never looks at the current residual, so unlike a robust loss it cannot mistake a genuine spot for an outlier while the starting geometry is still far off and leave the fit unable to move. That failure is not hypothetical - a CauchyLoss on this same residual, at the scale the multi-frame GeometryRefiner uses, collapsed one crystal's indexing rate from 99.89% to 19.83% and was rejected. It does not work by telling good spots from bad, and it does not need to. No per-spot property separates spots that index from spots that do not: measured AUC is 0.53 for peak pixel, 0.53 for total intensity, 0.51 for pixel count, 0.45 for peakedness, and a logistic regression on all twelve available features with pairwise interactions reaches only 0.64. What the weight does is halve the EFFECTIVE COUNT of every spot (mean w^2 = 0.517), and the damage scales with the absolute count of unexplained spots in the objective - 80.6 per frame here against 36.8 for the finder that was never damaged. That is also why an empirical `--max-spots 66` cap works while leaving the list no purer than before: it reaches the same operating point by discarding spots. This reaches it without discarding any, and without a tuned constant. Rotation battery, 33 crystals, both spot finders: finder A 29/33 -> 30/33 point groups (one crystal P222 -> P4212 = XDS, its high-shell CC1/2 86.0 -> 98.4) finder B 28/33 -> 29/33 point groups (one crystal I222 -> I23, its high-shell CC1/2 14.8 -> 38.0) No crystal lost its point group in either mode and no run failed. On the meta-stable multi-lattice dataset the CC1/2 spread over four frame ranges falls 19.7 -> 13.1 for finder B, and the indexing rate rises in 8 of 8 configurations. The crystal that the rejected robust loss destroyed keeps its 99.89% indexing rate exactly. The cost, stated plainly: ISa falls by 0.2-1.7 on about five crystals (and rises on two). Point-group correctness is worth more than that - merging in the wrong symmetry cannot be undone from the output, whereas ISa is a quality metric of data that remain correct - but it is a real trade and not a free win. Off by default. The indexers pass a spot list they have already selected, so their calls are unchanged; only the per-image refinement, which gets the raw list, turns it on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-28 09:53:13 +02:00
leonarski_fandClaude Opus 5	ec7a826136	Rotation scaling: guard the fulls refit against a collapsed per-frame scale too ... 704098712 guarded the per-frame scale on the partials, but it put the check inside ComputeSmoothGWindow - which only the partials path calls. Step 4 refits the scale from scratch on the COMBINED FULLS (Unity model, so corr is exactly 1/G), with no smoothing and no floor, and that refit was still free to collapse toward zero. It is the same failure and it is worse here, because there is no window average to dilute it: the collapsed frame's own fulls are multiplied directly. Measured on a dataset where the previous commit had already fixed the partials stage, the fulls refit put 1/G = 559x and 175x on two frames carrying 517 observations, and the merged CC1/2 read 26.2% where the intensities ENTERING that stage were fine - better, in fact, than the comparison run's in all ten resolution shells (R_meas 34.5% vs 39.5%, CC1/2 89.1% vs 83.0%). Reject a collapsed scale here as well, on the same measured criterion, and let those fulls merge unscaled - the state the combine left them in, and the same fallback the fit already uses for a frame with too few reflections. The check reads the host fulls after both the CPU loop and the GPU ScaleFulls, so one implementation covers both paths; the corrected corr is pushed back to the device exactly as the correction surfaces already do. THIS IS NOT A DETECTION-MODE PROBLEM. Over 8 configurations (both spot finders x 4 frame ranges) the separation is exact: every run with a collapsed scale had CC1/2 <= 58.5%, every run without had CC1/2 >= 74.1%, and nothing else predicted it. On one frame range it is the DEFAULT finder that collapses (CC1/2 58.5%) while the other is clean at 93.8%. The instability was never specific to the finder; it was latent in the scaling stage and either finder could trip it. Same 8 configurations, with this commit: finder A full 93.7 -> 93.7 (untouched) finder A -s 1 58.5 -> 91.3 (recovered) finder A -e 899 94.0 -> 94.0 (untouched) finder A -e 898 87.2 -> 87.2 (untouched) finder B full 26.2 -> 91.1 (recovered) finder B -s 1 93.8 -> 93.8 (untouched) finder B -e 899 8.1 -> 91.7 (recovered) finder B -e 898 74.1 -> 74.1 (untouched) Every collapse recovers; every healthy run is unchanged. The CC1/2 spread over the four frame ranges falls from 35.5 to 6.8 points for one finder and from 85.7 to 19.7 for the other - this dataset was not sampling a deep instability when its CC1/2 swung between 17 and 94 across frame ranges, it was sampling whether this bug happened to fire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-27 23:08:50 +02:00
leonarski_fandClaude Opus 5	7040987125	Rotation scaling: do not trust a per-frame scale that has collapsed toward zero ... The per-frame scale enters every intensity as 1/G, and SolveScaleIRLS floors G at zero and nothing else. A frame whose fit is not determined by its data can return G ~ 0.002 against a run median of 0.865, and every observation it carries is then multiplied by ~500 - sigma by the identical factor, which is why no sigma-based outlier test can see it and why this looked for a long time like a partiality problem. (The 1/partiality path is in fact guarded: min_captured_fraction floors it at 0.7 by default on rotation.) The window smoothing that should have absorbed such a frame instead made it permanent. It averages log G over a window, so a scale collapsing toward zero does not merely corrupt its own frame - its logarithm drags the whole window down. Worse, where a run has a stretch of frames too sparse to fit at all, the only FITTED frames in a window can be the collapsed ones, and the geometric mean then averages the fault with itself. Measured on a multi-lattice dataset: frames 816 and 818 fitted G = 0.0023 and 0.0014 with every neighbour from 800 to 839 unfitted, so smoothing set G = 0.0018 across the whole neighbourhood - a 546x amplification. About 500 observations of 152000 (0.66%) then carried 99% of sum(I^2), and the merged CC1/2 read 17.2% where the same data with the classic finder read 93.7%. Treat a fitted scale far below the run's median as what it is - an undetermined scale, exactly like the too-few-reflections case the code already handles - rather than as a successful fit. Such frames no longer contribute to the smoothing mean, and a frame whose own scale is not credible takes the neighbourhood's, or the run's typical scale when the neighbourhood holds nothing credible either. The bound is a RATIO to the run's own median because the rotation per-frame G is not gauge-fixed: G and the group means have an exact global multiplicative degeneracy, and the fitted median drifts over 0.745-1.358 across the battery. An absolute floor would reject everything in a run that drifted low. MIN_CREDIBLE_SCALE_RATIO = 0.02 was chosen from measurement over 12 crystals in the default configuration, where the smallest legitimate min(G)/median(G) is 0.070; the failing case sat at 0.0017. It is 3.5x below anything real and 12x above the failure. Effect on the intensity tail of the failing case: max I 10224 -> 438, and the top 1000 observations' share of sum(I^2) 0.990 -> 0.421 (the classic-finder reference is 0.632, so the tail is now cleaner than the run this was compared against). Rotation battery, 33 crystals in the default configuration: ZERO crystals differ - no space group, CC1/2, high-shell CC or ISa change anywhere. The guard fires only on the pathology. It does NOT rescue that dataset: with the amplification gone its CC1/2 is 26.2% and R_meas 49.2% against the classic finder's 93.7% and 27.8%. Adaptive detection degrades those intensities for a second, independent reason that is still open. This commit removes a latent hazard for any run with a sparse stretch of frames; it is not the fix for that dataset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-27 22:43:58 +02:00
leonarski_fandClaude Opus 5	0cd8cb7ba3	Space-group search: name the veto that actually refused a promotion ... The refusal message fell through to the chi^2 branch whenever the systematic-b balloon veto was the binding test, so it reported a chi^2 ratio that did not justify the refusal at all - on one battery crystal it printed "merge chi^2 is 1.25x the subgroup's (bound 1.85)", i.e. a number comfortably inside its own bound, as the reason for processing in the lower symmetry. A diagnostic that names the wrong cause is worse than none: it sends the reader after the wrong statistic. Report the b test when it is what fired, with both b values and the bound. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-27 21:38:07 +02:00
leonarski_fandClaude Opus 5	3171b071e6	Space-group search: judge a promotion against its parent, not against the error model ... The point-group decision moved with the AMOUNT of data at fixed physics: a partially twinned trigonal crystal was promoted into the twin's holohedry whenever the search happened to see a larger first-pass merge, and kept its true subgroup when it saw a smaller one. Simulation over 6 noise draws with only the merge multiplicity varying: the twin is promoted 0/6 at multiplicity 2 and 6/6 at 18, while the genuine control is promoted 6/6 throughout. The cause is that every existing gate is a ratio to the merge error model - b_parent grows toward the true systematic scatter as sigma shrinks with 1/sqrt(N), while b_cand is already saturated by the twin's disagreement, so the ratio slides down through a fixed veto. The parent statistic moves with data amount and the candidate statistic does not. Gate promotions on the operator disagreement H = <|I1-I2|/(I1+I2)> instead, as the ratio of the operators a promotion ADDS to the parent group's own operators on the same reflections. There is no sigma in it, so it cannot drift with the error model, and the parent normalisation cancels data quality. Measured over 27 runs, 5 promotion types and 450-1800 images: genuine symmetry 0.862-1.219, merohedral twins 1.270-2.084. On the synthetic grid it is flat across a 9x change in multiplicity - genuine pinned at 1.00, twins 3-12x the bound - which is precisely the property the old gates lacked. chi^2 and the systematic-b stay as secondary vetoes; they protect against non-crystallographic pseudo-symmetry, which is where correlation-based scoring is weak. Pick the parent carefully: 422 has two maximal subgroups of order 4, and on a tetragonal crystal twinned by 2[100] the rival (222) is CC-confirmed too and CONTAINS the twin laws, so normalising against it hides the twin among the promotion's own real operators (ratio 8.19 against the true parent, 0.78 against the rival). Where several parents tie, judge on the most damning. Also: - Report a refused promotion instead of silently processing lower. Merging a twin in the twin's holohedry averages non-equivalent reflections into each other and cannot be undone from the output; keeping the subgroup costs only redundancy. The refusal names the group and the number that caused it. - Stop the twinning report from arguing in a circle. It ran after adoption and conditioned on the adopted group, so a promotion into a holohedral Laue class made it print "no merohedral twin law exists" - the test was conditioned on the decision it should audit. Twinning is now also measured on the subgroup merge before adoption, and the post-adoption text says when its own conclusion is not authoritative. - Compare PRIMITIVE cell volumes in the first-pass scheme tie-break. A centred setting's cell is an exact integer multiple of its primitive one (a rhombohedral lattice in hexagonal axes is exactly 3x), so the integer-supercell test fired on a pure setting difference and demoted a good scheme to a threefold-smaller merge - which is what let the twin see the small merge to begin with. Rotation battery, 33 crystals: point-group agreement 30/33 -> 29/33, one crystal moved. That crystal (P422 -> P222) is the one with the known unresolved integration defect where reflections near the rotation-axis plane are wildly mis-integrated; its symmetry mates genuinely disagree, and its lower-symmetry merge is measurably better (ISa 2.72 -> 3.63, high-shell CC 75.4 -> 86.0). The threshold was not moved to accommodate it: 1.25 sits inside the measured gap and widening it would admit real twins. Separately the tie-break improved one crystal's CC1/2 from 77.7 to 84.0. Tests: a synthetic twin-fraction x multiplicity grid, which is what the search had never had - the existing tests are noise-free and exercise only Stage B absences. A NOTE ON WHAT WAS TRIED AND REJECTED, so it is not rebuilt: the obvious "physics-anchored" statistic is the disattenuated cross-validated correlation rho = corr(I_half0(h), I_half1(Rh)) / corr(I_half0, I_half1), which is 1 for real symmetry at any data quality and 2a(1-a)/((1-a)^2+a^2) for a twin. It passes the synthetic grid perfectly and FAILS ON REAL DATA IN BOTH DIRECTIONS - five false refusals of genuine symmetry on the battery, and it waves through a twin (rho 0.998) that H refuses. The reason is that cc_half correlates the two halves of the SAME reflection and so measures only random error, while cc_cross compares DIFFERENT reflections carrying different systematic error; dividing by cc_half removes the noise and leaves a systematic floor that varies by crystal AND by operator. Genuine rho measures 0.9987 on strong data and 0.73 on weak. A synthetic generator validates a statistic's arithmetic, never its premise, and this premise - that the only departure from exact symmetry is noise - is false for every real crystal. Any per-operator agreement statistic needs a same-crystal reference; an absolute threshold on one cannot be made to work by tuning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-27 21:05:16 +02:00
leonarski_fandClaude Opus 5	c9b52857e0	rugnux: warn when --min-image-cc is ignored; drop a dead robust estimator ... --min-image-cc is consumed only by the stills merge (MergeOnTheFly); RotationScaleMerge never reads it. On rotation data it was accepted and then silently did nothing, so a run that looked filtered was not. It now says so. FitProfileRadius_MAD had zero callers - a robust twin sitting uncalled next to the non-robust estimator that is actually used is a trap, so it goes. Neither changes any result: verified on a rotation dataset (indexing rate, cell, space group and merge statistics identical, warning emitted). Context for anyone tempted to wire that estimator in: I tested exactly that today and it is NOT justified. The population it would clip is truncated by construction - a spot is only marked `indexed` when its fractional-Miller norm is inside the indexing tolerance - and is measurably shorter-tailed than Gaussian (kurtosis 2.85). Across four serial-stills datasets a MAD-clipped variant only narrowed the prediction window (-17% integrated reflections everywhere), which was neutral on strong data and destroyed real signal on weak data (one set lost completeness 96.0 -> 93.9%), with R-free 0.3753 -> 0.3767. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-27 10:46:52 +02:00
leonarski_fandClaude Opus 5	16bf3408f0	Address code-review findings; make detection limits detector-driven ... One changeset, developed together in response to a review of this branch, so the files carry several of the changes at once. Full test suite passes (733 cases). Spot finding - Split ImageSpotFinder into Detect() (flag strong pixels - the expensive per-pixel pass) and ExtractSpots() (CCL + min/max-pix + resolution mask), with Run() = both. The per-image min-pix escalation now detects ONCE and repeats only the cheap extraction, instead of re-running the whole finder four times per frame as it did on the default path. It also keeps the winning attempt's spot list rather than re-extracting it, so the frame that is integrated is exactly the frame that was scored - which a GPU re-extract could not guarantee (float atomic ordering). - spot_finding_time_s no longer swallows indexing time, and indexing_time_s now sums every escalation call instead of reporting only the last. Detection limits follow the detector - The azimuthal-integration upper q and the spot-finding high-resolution limit are now std::optional, in the C++ structs AND in the OpenAPI schema, and resolve to the detector's own maximum (DiffractionExperiment::GetDetectorMaxQ_ recipA). Adaptive detection reads a pixel's ring from the azimuthal bins, so a pixel outside that q range could never be strong - the integration range silently bounded what detection could see, regardless of the requested resolution limit. Regenerated the C++ and TypeScript clients; the viewer and the web frontend each gained a "to detector edge" switch. Detection defaults are now per workflow (measured, not assumed) - Stills: adaptive detection, min-pix chosen per image, no resolution clipping. - Rotation: fixed-threshold finder, min-pix 2, 1.5 A limit. On a 33-crystal rotation battery, adaptive detection helped four hard crystals but deterministically broke three (a lost space group, a halved indexing rate, a collapsed merge), and the detector-edge limit cost indexing on a strong rotation set (100.0 -> 96.8%). Each is still overridable by its flag, and --no-adaptive-spots is new. Indexer seed escalation - Stop escalating once a seed's lattice explains >= 90% of the seed spots. Previously any frame with >= 80 spots always paid three indexer calls, online broker included. Merge-consistency filter - --min-image-cc gated on a per-image CC computed BEFORE the stills partiality post-refinement and never refreshed; the refiner now recomputes it, so the reported CC describes the data that are actually merged. - Replaced the per-call cc_mask argument with one MergeOnTheFly flag, so the merge, the error model and MergeStats can no longer disagree about which images are in (the --scale path merged unfiltered while its statistics were filtered). Per-image B-factor refinement (-B) removed - Measured on four serial-stills datasets: it is a no-op where the per-image fit is well conditioned and actively harmful where it is not (CC1/2 -8.1, R_meas +23.2 on the weakest large-cell set, whose fits hit their [-50, 200] bounds on 14-25% of images). It had also been silently DISCARDED since the partiality post-refinement landed - reported but not applied. Rather than fix and keep a knob with no demonstrated benefit, the flag and the whole image_scale_b_factor chain are gone: setting, scaling fit, message field, CBOR, HDF5 write and read-back, per-image plot, OpenAPI enum, viewer column and checkbox, docs. ScaleOnTheFly no longer needs Ceres at all - the fit is a linear IRLS. (The Wilson per-image b_factor is a different quantity and stays.) Stills partiality width now fits both of its components - sigma^2 = gamma0^2 + (gamma_e*d*)^2 instead of a purely angular gamma_e*d* with gamma0 pinned to 0. Fitted per crystal by least squares of dist_ewald^2 on d*^2. The angular-only width is fitted over a d*^2-dense population, so it was pinned by the high-resolution edge and collapsed at low d*: median partiality 0.008 beyond 13 A for reflections that were plainly recorded, 55% of them under the merge's partiality floor, and the survivors divided by those values - which inflated the merged low-resolution intensity scale 3.6x (~ +9 A^2 of apparent B). Measured on 5000 stills: the ramp flattens to 0.89x, no observation is dropped any more (701750 -> 716811), shell-mean CC1/2 and R-free improve slightly. Note CC1/2, R_meas, completeness and a B-refining R-free are all blind to that ramp, which is why it survived earlier validation; the cost is high-resolution R_meas (98.5 -> 101.9 shell-averaged). Removed dead code from add-then-remove churn - Prediction-time "still partiality" (unreachable: no setter), the phantom IndexingSettings::min_indexed_spot_fraction knob (getter, no setter - now the constant it always was), StillsPartialityRefine's caller-less Settings constructor and its reference to a long-gone env var, ProcessImage's unread bool return, an unused include, and a dead viewer overlay hook. Also - Viewer: the magnifier compared a QImage with itself, so its scene rect was set once ever and it could not pan into a larger dataset; the hover tail timer could fire after leaveEvent and resurrect the resolution readout outside the image. - update_version.sh regenerated the frontend lock file BEFORE bumping the version (every release shipped an off-by-one lock), and did git rm/git add on a path that has not existed since the client moved to src/client - with no set -e, both failed silently. - fpga/pcie_driver/postinstall.sh tested "[ ! occurrences > 0 ]", which is a redirect, not a test, so dkms add never ran. - Unit tests for the adaptive-threshold host functions, which had none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-27 09:07:00 +02:00
leonarski_fandClaude Opus 5	38f1c1a387	Revert "Viewer: read the data-analysis algorithm documentation from Help" ... This reverts commit 681e80e46. Dropped for now: QTextBrowser::setMarkdown leaves the inline LaTeX in CPU_DATA_ANALYSIS.md as raw "$...$" source, and the document's own HTML/math rendering wants sorting out first. Reverted rather than rewritten so the implementation stays available in history to bring back afterwards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 22:08:45 +02:00
leonarski_fandClaude Opus 5	6f15ae04b7	Viewer: read the data-analysis algorithm documentation from Help ... Adds Help > Data Analysis Algorithms, showing docs/CPU_DATA_ANALYSIS.md in a window. The document is baked into the binary through the Qt resource system (aliased to :/cpu_data_analysis.md), so it needs no docs/ directory at runtime and cannot drift from the build it shipped with. Same shape as the existing third-party licences window, created once and raised thereafter. Limitation worth knowing: QTextBrowser::setMarkdown renders the headings, lists, emphasis and inline code well, but it has no math support, so the inline LaTeX in the more quantitative sections appears as raw "$...$" source. The descriptive material - which is most of the 744 lines - reads fine. Fixing that properly means either pre-rendering the document to HTML with a math filter at build time, or sending the user to the Read The Docs copy instead; neither seemed worth doing without knowing which you would prefer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 22:05:11 +02:00
leonarski_fandClaude Opus 5	79b86164a4	Viewer: ROI statistics appear as soon as the first ROI is drawn ... Drawing an ROI reported Sum 0 and Max 0 until the next frame was loaded, with only the pixel count looking right because that comes from the ROI map rather than from the data. SetROIDefinition_i called RunROIOnly, which integrates whatever the preprocessor buffer already holds. LoadImage_i only preprocesses an image when a ROI is already defined, so the very first ROI is drawn on an image that was never preprocessed: the buffer is empty and every sum integrates to zero. From the next frame on a ROI exists, LoadImage_i preprocesses, and the numbers look correct -- which is what made this look like a refresh problem rather than a wrong call. Use AnalyzeROIOnly, which preprocesses the image before integrating. It costs a pass over the image per ROI edit, of the same order as one recolour, and ROI edits already keep at most one recompute in flight (live_pending_ in JFJochDiffractionImage), so the editing rate is bounded. I did not measure the drag rate specifically. Verified with a single frame loaded and no frame step: the first ROI drawn over the beam centre reports Sum 66065, Max 2761, Mean 0.473, centre of mass (787.3, 844.5). The Max equals the image's own reported maximum of 2761, as it must for a box containing the brightest pixel. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 21:49:02 +02:00
leonarski_fandClaude Opus 5	84a1538495	Viewer: drop the in-view ROI accumulation, the worker already does it ... The statistics shown for a drawn ROI do not come from the view at all. Drawing one promotes it to a named ROI, roiGeometryEdited goes to the reading worker, and the worker's per-image results arrive in ImageData().roi, which is what the Inspector's ROI section displays. So accumulateROI/CalcROI/roiCalculated were a second implementation of the same thing whose output nothing read -- and the worker's version is the better one: it handles the mask and it persists per image. Remove them. The view now owns only the ROI's geometry and gestures, which is all the worker needs from it. This corrects the previous commit's claim that nothing surfaces ROI statistics: the Inspector does, via the worker. Verified by drawing a box over the beam centre: Sum 65453, Max 1634, Mean 0.468, centre of mass (786.3, 843.4) against a beam centre of (764, 850). Note the numbers appear from the next analysed frame onward, since the worker attaches them at analysis time and the displayed frame was analysed before the ROI existed -- that behaviour is unchanged here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 21:42:30 +02:00
leonarski_fandClaude Opus 5	57f9e42382	Viewer: the region of interest belongs to the diffraction view alone ... Drawing an ROI only means something where there are detector counts to accumulate. Gate the gesture on a virtual AllowROI(), true only for JFJochDiffractionImage: shift-drag, the resize handles, the hover cursor and the "Clear ROI" context entry now do nothing in the azimuthal, grid-scan and calibration views, which cannot report anything about a box anyway. The statistics move out of the base class into the diffraction view and read the int32 image directly, so no float copy of the detector image is built for them either. With the labels already converted, image_fp is now untouched by the diffraction view, and the lazy EnsurePixelValues machinery it needed is gone. image_fp stays as the base's representation for the views whose data really is float: the azimuthal profile, the grid-scan 1/sigma^2 map, and the calibration viewer's eight source types. Removed with it: the ROI readouts in the calibration and 2D azimuthal windows, which were the only two consumers of roiCalculated -- the diffraction view emitted it and nothing listened. Nothing surfaces ROI statistics now; the pixel-mask case wants rectangles counting excluded pixels and deserves its own design. JFJochViewerROIResult is still used by the side-panel ROI list, so the widget stays. Verified in the GUI: shift-drag in the diffraction view still draws the box, turns it into a named ROI and runs the statistics; fit-view panel remains pixel-identical to the pre-series baseline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 21:32:47 +02:00
leonarski_fandClaude Opus 5	27615a8a1d	Viewer: label pixels from the int32 image, and paint them instead of building items ... Two changes to the per-pixel value labels, which appear above 30x zoom. They were up to 5000 QGraphicsSimpleTextItems created and destroyed on every overlay rebuild - so on every pan step while zoomed in. Paint them in drawForeground() instead: no item churn, no scene invalidation, and the text is laid out in viewport pixels so it is a constant readable size rather than a scene-space font scaled by 0.2. Same approach as the magnifier's labels. The value text becomes a virtual, PixelLabel(). The base still formats from image_fp, which is what the genuinely float-valued views hold (azimuthal profile, grid-scan 1/sigma^2, the calibration viewer's eight source types). JFJochDiffractionImage overrides it to read the int32 image directly: counts are exact integers, so routing them through float32 is a detour that also cannot represent summed values above 2^24 exactly. Verified at 38 wheel clicks over a module edge: identical values and gap/contrast handling to the previous float path, now centred in each pixel. Fit-view panel still pixel-identical to the pre-series baseline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 21:27:17 +02:00
leonarski_fandClaude Opus 5	7a893bb1e7	Viewer: per-pixel counts in the magnifier, read from the int32 image ... Users expect a magnifier to tell them the counts, which the follower view could not do: it has the rendered pixels but not the numbers behind them. Take them from the detector's int32 buffer directly, the same source the main view colours from, so no float copy of the image is needed - the magnifier still holds nothing full-size of its own, only a shared_ptr to the frame and one to the reader image. The labels are painted in drawForeground() rather than as scene items. The main view creates up to 5000 QGraphicsSimpleTextItems per overlay rebuild for this; here they are just drawn, so there is no item churn and no scene invalidation. Text is laid out in viewport pixels so it stays a constant readable size, and black/white is chosen from the luminance of the rendered pixel underneath, as the main view does. Threshold is the same 30x as the main view, so the default 12x magnification shows no labels until the user wheels in; a cap keeps pathological window sizes from drawing thousands of them. Verified in the GUI at 32x: counts drawn per pixel with white text over the dark centre of a Bragg peak and black elsewhere, and "Gap" across a module gap. Main image panel still pixel-identical to the pre-series baseline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 21:18:20 +02:00
leonarski_fandClaude Opus 5	d2ce65f857	Viewer: magnifier displays the frame the main view already rendered ... The magnifier and the main view are two views of the same image at different position and zoom, but the magnifier ran the whole pipeline again on its own copy: it wrapped the same int32 buffer in a SimpleImage, converted it to float, coloured every pixel and kept its own full-size QImage. That is a second conversion and two extra full-detector buffers (20 MB at 2.8 Mpx, 138 MB at 18 Mpx) to feed a 320x320 window. Separate producing a frame from displaying one: - JFJochImage keeps the rendered frame in a shared_ptr<QImage> (the pointer is stable for the widget's lifetime; only the contents change, so the existing buffer reuse is unaffected), publishes it via Frame() and announces new pixels with frameRendered(). - JFJochImageItem holds that shared_ptr instead of a reference to a member of its owner, which also removes a lifetime coupling. - JFJochFollowerImage is a small read-only view of such a frame with its own zoom and centre. It shows only the image: overlays, ROI tools and per-pixel labels belong to the view that owns the data. - The magnifier becomes one of those, fed from frameRendered(). Consequences beyond the saving: the magnifier now agrees with the main view on colour map, contrast and HDR mode, which it never did -- it was wired to neither, so it always drew with its own defaults. And the visibility guard added in 6d1af4921 is gone: there is no longer any per-frame work to skip, so nothing needs guarding. That guard was a workaround for this design. Stepping 30 frames with the magnifier open: 5550 -> 4810 ms CPU, which is what it costs with the magnifier closed (4770 ms) -- it is now free either way. Verified: main image panel and a drag-pan stay pixel-identical to the pre-refactor binary (AE=0); the magnifier follows the cursor, updates on a new frame, and now tracks a colour-map change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 21:11:42 +02:00
leonarski_fandClaude Opus 5	5782cc0edf	Viewer: reciprocal-space view does nothing while its window is closed ... The window is a placeholder for future functionality and is closed almost all of the time, but it extracted the frame's spots and rebuilt and uploaded its vertex arrays on every image, whether or not anything was on screen. Guard it in rebuildGL() rather than at each of the eight call sites, so any future caller inherits the behaviour: while hidden it only records that a rebuild is owed, and showEvent() pays it. imageLoaded() additionally skips extracting the frame's spots, which is the other half of the per-frame work. The OpenGL code path is untouched and still built and exercised the moment the window is opened. Note: I could not show a CPU saving for this on the headless test machine -- there, ~74% of the process CPU is Mesa llvmpipe software rasterisation that I was unable to attribute to any per-frame code path, and it swamps the effect. The work being skipped is nonetheless unambiguously unnecessary. Verified in the GUI: after stepping frames with the window closed, opening it shows the current frame's spots, and it keeps updating while open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 21:05:21 +02:00
leonarski_fandClaude Opus 5	68f5f1f32d	Viewer: do not render the magnifier close-up while it is closed ... centerAt() checked isVisible(), but imageLoaded() did not, so every frame built a SimpleImage over the whole detector image and ran it through the full JFJochSimpleImage path -- convert to float, colour every pixel, redraw -- to feed a 320x320 window that is closed by default and stays closed most of the time. Remember the frame instead and do the work in showEvent(). Holding the shared_ptr also keeps alive the buffer that the SimpleImage's CompressedImage points into, which it did not own. Stepping 30 frames with the magnifier closed: 5545 -> 4770 ms CPU (-14%), on a 2.8 Mpx detector; the saving is per-pixel, so it grows with detector size. With the magnifier open the cost is unchanged (5500 ms), which is what was being paid unconditionally before. Verified in the GUI: opening the magnifier still populates it, and it still refreshes when the frame changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 20:45:11 +02:00
leonarski_fandClaude Opus 5	2a31cf8d81	Viewer: stop forcing FullViewportUpdate in JFJochSimpleImage ... FullViewportUpdate redraws the whole viewport on any change. The attached comment ("keep overlays in pixel units independent of zoom") does not describe what the setting does, and nothing here needs it: SmartViewportUpdate repaints the changed rectangles and falls back to a full repaint by itself once there are too many to be worth tracking. This is the view used by the calibration window and the magnifier, and the magnifier is driven from every hover, so on a remote session it repainted its whole viewport per pointer motion. Note: not exercised visually -- both windows are opened from menus, which the headless harness does not drive. The change is a repaint-mode switch with no effect on what is drawn. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 20:13:21 +02:00
leonarski_fandClaude Opus 5	4892c57119	Viewer: paint the resolution readout in drawForeground, not as a scene item ... The hovered "d = ... A" readout was a QGraphicsTextItem flagged ItemIgnoresTransformations, repositioned on every mouse motion. Qt cannot compute a tight dirty rect for an item that ignores the view transform, so it marks the entire viewport dirty whenever such an item moves or changes text -- and this one moved constantly. Paint it in drawForeground() in viewport pixels instead, and repaint only the union of its old and new rectangles. That also removes the item lifetime special-casing: it was deliberately kept out of overlay_items_, had to be nulled by hand after scene()->clear(), and carried comments in three places warning about the dangling pointer. This does not reduce raw X11 traffic -- there every repaint uploads the whole window whatever the damage -- but it cuts the work per hover, and it does matter under a compressing remote protocol (VNC/NX/xpra), which encodes only the region that actually changed. Verified against the previous build: same text, colour and position. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-26 20:13:04 +02:00