v1.0.0-rc.170 #80

Merged

leonarski_f merged 17 commits from rc170 into main 2026-09-16 18:17:47 +02:00

Conversation 0 Commits 17 Files Changed 185

+1151 -469

17 Commits

Author	SHA1	Message	Date
leonarski_fandClaude Opus 5	a48d26dd92	rugnux: the rescue ladder finds spots once per floor, not once per rung ... The first-pass ladder's twelve rungs are four indexer-side spot caps taken at three resolution floors, but the loop varied the floor fastest and apply_state clears the spot cache unconditionally, so every rung re-ran spot finding over all sixty validation frames - and the cap is applied by the indexer over the cached lists (MaxSpotsPerImage), so nine of those twelve finder passes produced byte-identical lists. Group the rungs by the finder-facing settings (rings, d_min) and re-find only when those change; a seed change just moves the cap. Adoption is decided over all rungs at once, so the visit order carries no meaning and the chosen rung is the same. On a no-crystal dataset - where every rescue fires and answers 0/60 - the run drops from ~4 min to 1:49; datasets the ladder genuinely rescues pay three finder passes per invocation instead of twelve. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 16:13:48 +02:00
leonarski_fandClaude Opus 5	968d456a11	harden the paths today's review flagged: launches checked, inputs bounded ... From the five-agent review of rc168-rc170 and the ticking-bomb hunt: - Every CUDA kernel launch is followed by cuda_err(cudaGetLastError()) (or the file's own check idiom) - 53 launches in 11 files ran unchecked, so a non-sticky launch failure (out-of-resources on a shared GPU, a zero grid) silently handed stale device buffers downstream as good data. The FFT indexer got this check in bfe95b4ed; this is the same gap everywhere else. BeamCenterFFTGPU already checked every launch through CheckLastKernel. - ShadowFinder: a non-finite or absurd beam centre is refused before it can become a negative ring index (an out-of-bounds write) or an arbitrarily large per-ring table; a pixel whose polarization correction is not strictly positive is not usable - divided by zero it put an inf into the pooled means, which the running box sums turn into NaN for a whole row. - FileWriter: the network-supplied image number is bounded by the collection's declared number_of_images - unbounded it sized per-image vectors, a huge value was a fatal allocation and a wrapping product an out-of-bounds heap write. - ROICircle/ROIAzimuthal: parameters must be finite, not merely positive - NaN passes every <= test, inf passes > 0, and both reached the preview drawing where a non-finite loop bound hangs the rendering thread. - Reader + viewer: documented that SWMR / growing HDF5 files are not supported - a file we open is expected to be final, which is why re-opening the currently open path deliberately does not re-read it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 16:01:55 +02:00
leonarski_f	6dc239aaa8	Merge branch 'refobs' into rc170 ... # Conflicts: # docs/CHANGELOG.md	2026-09-16 15:32:46 +02:00
leonarski_fandClaude Opus 5	53a1d3d2a6	docs: condense the rc170 changelog to one line per change ... Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 15:21:55 +02:00
leonarski_fandClaude Fable 5	29a52ad964	viewer: black spot outlines are opt-in, highest pixels marked by framed boxes ... The black casing rc169 put under every spot marker made the markers harder to read for some users, so it is now off by default; the "Outline spots in black" checkbox under "Image features" switches it on, and with it off the spots draw exactly as before rc169. The N-highest-pixel markers change from feature-coloured crosses to a white frame in a black casing: a frame points at the pixel inside it, where a cross sits on top of the very pixel it marks. The frame is slightly smaller than a spot frame and drawn above it, so both stay visible when a top pixel falls inside a spot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-09-16 15:13:15 +02:00
leonarski_fandClaude Opus 5	d752a78ee5	docs: the rc.170 memory line reflects the further scale/merge reduction ... Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 14:37:26 +02:00
leonarski_fandClaude Opus 5	fd274d5bda	scale/merge: the resident ingest never materialises the full Obs record ... The previous commit freed the host Obs array once the device held every field; this one stops building it. With the GPU pipeline resident (and no observation dump) the build lays down a forty-byte record - hkl, frame, rocking position, resolution, zeta, delta_phi, partiality, and the rocking-event usability bit - holding exactly what the host still touches between the build and the release: the geometry smoothing and the partiality recompute, the corr snapshot, and the rocking-event median. Every immutable per-obs field is staged to the device straight from the source reflections, in frame-aligned bounded slices, and the recomputed partiality follows after the smoothing. The smoothing loops and the rocking-event walk run on either record through the same code (templated on the record type), so the arithmetic is exactly the one the full record ran; the CPU and dump paths keep the full Obs as before. On a rotation crystal with a ~640 A axis and 95.7 M first-pass partials the full record was 7.7 GB, built and held beside the 8.4 GB of source reflections it copied from; the narrow one is 3.8 GB and is handed back at the end of Ingest. Measured there, on top of the previous commit's release: peak RSS 20.3-22.3 GB -> 18.3 GB, and the phases that used to carry the peak drop far more - the second pass's post-refine probe and space-group search run at 13-15 GB where they ran at 18-20, and the first pass's post-Ingest plateau at 12-16 where it ran at 20-21. What remains on top is no longer the scale/merge: the new peak is the first-pass ingest sitting on the image loop's own high-water mark (~17 GB), which this code never touches. Byte-identical output on that crystal and on two ordinary rotation sentinels: merged, P1 and unmerged MTZ data blocks, .hkl, .cif, per-image table, and the report modulo date/git/command line/wall time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 14:37:13 +02:00
leonarski_fandClaude Opus 5	a1b63c3b5e	scale/merge: hand back the host copy of the partials once the GPU holds them ... With the device pipeline resident and no observation dump, nothing on the host reads the 80-byte Obs records after the upload: scaling, combine, scale-fulls and merge run on the GPU, the ASU grouping stamps a flat group_ids array, every pass restarts corr from corr_ingested, and the CPU combine only runs for the dump. The one late reader, RockingEventFrames, sees only ingest-time values on that path (the host corr is refreshed only in the dump fallback), so Ingest takes its answer up front and frees the array - the second full-size copy of the partial observations, held beside the source reflections for the whole of the canonical pass. On a finely-sliced long-axis rotation crystal with ~96 M first-pass partials that copy alone is 7.7 GB. Byte-identical by construction: every consumer that could ever read the array on this path is enumerated above, and the CPU and dump paths keep it exactly as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 14:37:13 +02:00
leonarski_fandClaude Opus 5	bfe95b4ede	analysis: no sort ever sees a NaN key, and FFT kernel launches are checked ... A production broker segfaulted inside std::sort in FilterFFTResults: a NaN length key violates strict weak ordering, and libstdc++'s unguarded partition scan then walks off the array (confirmed from the deployed binary's faulting instruction). No legitimate producer of that NaN exists - both FFT back-ends emit finite lengths - so the row was corrupted, most plausibly via the one gap in the path: neither kernel launch in FFTIndexerGPU::ExecuteFFT was error-checked, so a failed launch silently hands back uninitialised device memory. An audit of every sort/nth_element site then found two more places where a NaN key is reachable by construction. - FFTIndexerGPU: cudaGetLastError after both kernel launches (the idiom every other GPU translation unit already follows). - FilterFFTResults: drop non-finite rows when building the magnitude map; bit-identical in normal operation. - SearchSpaceGroup: PearsonCC deliberately returns NaN for an unscorable operator (n_pairs < 2, zero variance) and the score sort consumed it unfiltered - ~24 operators on a cubic holohedry is past the introsort threshold, the same crash waiting to happen. Unscorable operators now rank last under a well-defined comparator. - PostIndexingRefinement: a singular QR-solved cell puts inf into cell.inverse() and 0*inf = NaN into the residual norms fed to nth_element; non-finite distances now map to +inf, which says exactly "this spot does not index" and orders consistently. - ModelScaling: a NaN R factor from the first grid point latched into best_r and won every later comparison; it is now skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 14:05:49 +02:00
leonarski_fandClaude Opus 5	bef0c6f390	scale/merge: the memory peak no longer holds three copies of the partials ... The offline rotation pipeline held every partial observation in up to three forms at once at its peak: the per-image Reflection vectors (88 B/obs), RotationScaleMerge's flat Obs array plus its full-length GPU staging (80 + 57 B/obs), and the geometry post-refinement's own gather (2 x 32 B/obs while it bucket-sorts). On a rotation crystal with a ~640 A axis, 3600 finely-sliced images and 95.7 M first-pass partials that peaked at 28-30 GB of RSS and OOMed a 32 GB machine. Four reductions, none of which changes an output byte: * The post-refine gather scatters each partial straight into its h bucket (histogram pass first), so the gathered-order copy of the whole array - one of two full-size copies alive through the sort - no longer exists. * The gather is split from the fit (GatherPostRefineObservations), and the geometry pre-pass takes it right after Ingest, handing back each image's reflection vector as it is read: nothing later in the pre-pass reads them (its reflections are never written, and the scaling engine reads back only per-frame fields). A run with a reference MTZ keeps them, since resolving the indexing ambiguity re-reads them. * The pre-pass frees the whole RotationScaleMerge engine - about twice the partial payload again - before the post-refinement fit, which sat exactly on the pass's peak. The canonical pass keeps everything, as before. * Ingest's transients are bounded: the sort keys are consumed before the flat Obs array is built instead of coexisting with it, and the GPU upload stages bounded slices (under half a GB) instead of fourteen full-length arrays at once (5 GB on that crystal). Measured there: peak RSS 28.3 / 30.0 GB (two baseline runs) -> 20.3-22.3 GB (four runs of the fix), wall clock unchanged within run-to-run variance, and byte-identical output - merged/unmerged/P1 MTZ data blocks, .hkl, .cif, per-image table and report all equal, on this crystal and on two ordinary rotation sentinels (which also keep their wall clock and drop slightly in peak). What remains of the peak is the canonical pass, which cannot hand its reflections back (they are re-seated and written); going lower means removing the Reflection/Obs duplication itself, which this deliberately does not attempt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 11:53:50 +02:00
leonarski_fandClaude Opus 5	7dc066c0f3	model: the placed coordinates carry the group they obey, not the data's ... WritePlacedModel stamped the reflection files' space group on the placed model unconditionally. That is right whenever the coordinates obey that group - the ordinary case, and the subgroup case too, where the file just holds more than one asymmetric unit of it - but wrong when the data were merged in a supergroup of the model's group, or in the other hand. The extra operations are then a claim the contents contradict: a refinement program expands them, refines a structure that is not the model, and reports nothing. Measured on a public deposition whose data this run over-merged across a pseudo-symmetry twofold: refining the written pair gave R-free 0.495, where the same coordinates and the same reflections under the model's own group give 0.285. The label alone was worth 0.21 of R-free. The group is now taken from the argument only where every one of its operations is one the model already has, and the model keeps its own otherwise. The two files then disagree, which is the truth about them and is warned about, rather than agreeing on something false. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 11:52:24 +02:00
leonarski_fandClaude Opus 5	0080d2f4ba	viewer: wrap a long dataset name, centre the file-panel resize handle ... Word wrap never breaks inside a word, so a dataset name - one long token of underscores - was laid out on a single line and cut off at the panel edge; only a path with "/" in it wrapped at all. Break opportunities are now marked at "/" first, at "_" next, and every twelve characters of a run with neither, so the wrap uses the separators where they fit and breaks mid-name only where nothing else will. The file panel inset its contents by 4 px where every other dock sits flush, which put its edge 4 px from the resize handle it shares with the image and made the handle look off-centre; the handle between image and inspector was symmetric, and now both are. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 11:00:39 +02:00
leonarski_fandClaude Opus 5	65dd28744c	docs: changelog entry for the x-axis labels ... Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 10:46:39 +02:00
leonarski_fandClaude Opus 5	a37f72a011	viewer: label the whole x axis of a dataset plot, not the collected part ... The plot's x axis already spans the whole dataset, but its goniometer-angle ticks were computed from the images that had arrived, so a live run 10% in labelled its first tenth at ten times the granularity and left the rest of the axis bare. The goniometer gives the angle of an image before it is collected, so the ticks are now laid out over the full range - still snapped to round angles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 10:46:33 +02:00
leonarski_fandClaude Opus 5	f011af258f	docs: changelog entry for the viewer fixes ... Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 10:43:56 +02:00
leonarski_fandClaude Opus 5	5e43cea90d	v1.0.0-rc.170 ... Version bump: VERSION, the OpenAPI spec and its three generated clients, the docs, the frontend and the FPGA/driver version strings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 10:43:47 +02:00
leonarski_fandClaude Opus 5	f8f63fe913	viewer: inspector and dataset-plot fixes ... - The inspector's rotation row is labelled "Goniometer angle", and its Smargon chi/phi are printed to one decimal, with an angle that rounds to zero shown as 0.0 rather than -0. - The dataset name reserves three lines from the start, so the rows below it do not move as one dataset's name wraps further than the next one's. - The stacked spots/background plots share a plot-area left edge: Qt Charts puts the y labels inside that margin, so a five-digit spot count used to start its curve further right than a one-digit background. Each compact plot measures its widest y label and both are padded out to the larger. - The horizontal crosshair follows the cursor and snaps to the curve only within half a font height, so it can be used as a ruler anywhere on the plot instead of only reading back the value already in the status bar. - Over HTTP the Smargon position never reached the experiment: the CBOR start message carries it, but the reader dropped it, so a live session showed no sample-head angles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-16 10:43:09 +02:00