1281 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
leonarski_fandClaude Opus 5	abb94ca450	spot_finding: accumulate the adaptive ring statistics in integers ... The per-ring sums were floats reduced by atomics, so the ring sigma - and with it the detection threshold - depended on the order the blocks happened to arrive in. Detection compares an INTEGER pixel value against that threshold, so a threshold that drifts across an integer flips every pixel of that value in the ring at once, which is how a last-bit difference turned into a different spot list. A preprocessed pixel is an exact int32 and the masked and saturated sentinels are skipped, so v and v*v are exact in 64 bits, and integer addition is associative: the sums no longer care about arrival order. Both engines now accumulate the same way, so they agree exactly rather than approximately, and the GPU spot list is bit-identical across runs. The corrected sums that feed the reported azimuthal profile stay float - a pixel value times a float correction has no exact integer form - but they do not enter the detection decision. Cost: the ring reduction needs 28 bytes per bin instead of 20 in the plain pass, which drops it from eight co-resident blocks per SM to seven and costs about 11% of that kernel (0.582 -> 0.650 ms/frame on a 4.5 Mpx frame). End to end it does not show: alternating runs on three rotation crystals came out the same or slightly faster, and the battery is unchanged in every number. The CPU engine got 30% faster (32.2 -> 22.6 ms/frame), integers being cheaper than doubles. Tests: exact CPU/GPU agreement on the spot list, and 50 repeats of bit-identical output where there were four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 15:19:11 +02:00
leonarski_fandClaude Opus 5	212fbf9bab	rugnux: make the geometry-refinement sample deterministic, and spread it over the run ... The stills first pass drew frames from a shared cursor and stopped when a shared counter reached its target, which got two things wrong at once. The cursor walked the equally spaced sample in ascending order, so stopping early read only its leading PREFIX - the beam centre, distance and cell were fitted to the beginning of the run, not across it, and the comment claiming otherwise was wrong. And where the stop landed depended on how the workers happened to interleave, so the set of frames varied run to run: on the same data at -N 32 and -N 8 the pass examined 483 and 457 frames and refined the detector distance to 168.0481 and 168.0530 mm. The sample is now cut into a fixed number of interleaved stripes, each stopping once it has contributed its share. Every stripe spans the whole run, so an early stop no longer biases the fit, and a stripe is processed identically whichever worker claims it - so what gets examined depends only on the data, not on timing and not on -N. The same three runs now give 451 frames examined and 168.0452 mm, identically. The bundle selection was order-dependent too: frames are collected in worker-completion order and sorted by spot count with a non-stable sort, so equally strong frames swapped places between runs. They carry their image ordinal now and it breaks the tie. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 15:19:11 +02:00
leonarski_fandClaude Opus 5	5f8d37cdab	image_pusher: serialise tearing a connection down ... Two futures per connection are joined from more than one path: the acceptor reaping a dead connection, and the control plane starting or ending a run. Calling get() on one future from two threads at once is undefined and invalidates it, and the guard against it was a non-atomic test-then-set of an atomic flag - `if (!c.active) return; c.active = false;` - so both callers could pass it. Holding connections_mutex across the joins is what the previous commit removed on purpose, and rightly: the joins block on a writer that may be inside a send. So the lock is per connection and covers only the teardown. Nothing it joins takes it, so it cannot deadlock. The comment claiming a detached connection is unreachable by anyone else was wrong: a control-plane call that copied the pool before the erase still holds a shared_ptr to it. That is precisely how the two teardowns meet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:19:27 +02:00
leonarski_fandClaude Opus 5	2d3c39c9dd	image_preprocessing: write whole elements out of the un-transpose ... The raw-bytes path assembled each element a byte at a time, which on a full frame cost about 4x against writing the 8 contiguous elements a thread owns through an element-typed pointer. They are 8*ES-byte aligned, so the compiler merges them. 72.4 MB frame: 1.524 -> 0.406 ms for upload plus both kernels. The test now also times the LZ4 pass on its own, so the bounds and validity checks in the hot loop can be costed rather than guessed at. They are free: 0.231 ms against 0.2297 ms measured for the kernel before any of them existed - the restored offset == 1 and power-of-two fast paths pay for them. compute-sanitizer memcheck reports no error over 400 single-bit-corrupted payloads and nine malformed containers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:16:58 +02:00
leonarski_fandClaude Opus 5	b6c4a59d69	docs: record the stored-format break, and say how the adaptive finders actually accumulate ... The per-image image-scale B factor was dropped from the CBOR stream and from the written HDF5, which is a change for anything reading those files, but the changelog listed it only under the OpenAPI breaking changes. The GPU adaptive finder test claimed both finders sum the rings in double. The CPU one does; the GPU one stages a block's contribution in float before reducing across blocks in double, deliberately, to keep the hot loop's shared footprint down. Say so, and say what follows from it - detection compares integer pixel values, so a threshold that crosses an integer flips every pixel of that value in the ring at once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:14:24 +02:00
leonarski_fandClaude Opus 5	4b1c611bdf	image_analysis: query the current device, and upload the resolution mask on the engine stream ... Two leftovers from earlier fixes of the same shape. BraggIntegrationEngineGPU still read device 0's shared-memory size to decide whether its profile grid fits; workers are pinned round-robin across GPUs, so on a heterogeneous node that check can pass on a different card than the one the kernel launches on. SpotExtractorGPU still uploaded its default resolution mask with a pageable copy on the NULL stream, which is not ordered against the engine stream now that streams are created non-blocking. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:14:24 +02:00
leonarski_fandClaude Opus 5	32ecc296b8	broker: bootstrap a concrete max_hkl, as the API and the docs already claim ... How far the predictor walks the lattice became a setting defaulting to "derive it from the refined cell", which is what is wanted offline. Online it is not: per-image prediction cost then depends on whichever crystal is mounted, and the cube (2n+1)^3 grows quickly. The broker was only ever handed a concrete value when a client sent a bragg_integration block, and none of the shipped configs has one, so the derived path was the normal deployment. Four places in the tree - the conversion, the predictor, the OpenAPI description and the changelog - already state that the broker keeps a fixed bootstrap. Make that true. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:14:09 +02:00
leonarski_fandClaude Opus 5	42eb5bbe83	rugnux: clamp the refinement thread count to what the setting accepts ... RefineThreads rejects anything above 64, and the two-pass rotation path - the default - passed nthreads/2 straight from the machine. On a host with 130 or more hardware threads rugnux therefore exited 1 with the bare message "Candidate-cell refinement thread count" before reading a single image, and the only workaround was to discover -N and pass a smaller value. Dual 64-core parts are squarely in range. The limit is now a named constant the setter and the callers share, so the two cannot drift apart again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:14:09 +02:00
leonarski_fandClaude Opus 5	bec7e2e922	image_preprocessing: fuse the bitshuffle inverse with preprocessing, and verify the decode ... The device decoder was byte-exact on every valid input - 994 production-compressed images, 927 hand-built LZ4 blocks covering engineered (offset, matchlen) pairs across the overlap branch boundary, 18000 repeat decodes, sanitizer-clean - and an audit against LZ4_decompress_generic could not construct a valid block it mis-decodes. What it did not do was notice when the input was NOT valid, and that mattered more than it looks: the decode buffers are reused frame to frame, so a block that stopped early left the PREVIOUS image in place, and in the bitshuffled layout the untouched tail is the most significant byte-plane. A corrupt chunk therefore did not look like a missing corner. It looked like thousands of real pixels several powers of two too bright, fed to spot finding with no diagnostic, where the host decoder had raised an error. So the kernel now flags a block that fails to reach its declared length while consuming exactly its payload, and the host turns that into an exception once the caller has synchronised. Reads are clamped against the end of the payload as well as the output, both length chains are bounded exactly as read_variable_length bounds them, the two offset bytes are bounded, and LZ4's parsing restrictions are enforced. On the host side a block size that is not a multiple of 8 elements is rejected (it made the un-transpose read uninitialised shared memory), the block count is bounded by what the chunk could hold before it becomes an allocation (twelve header bytes could demand hundreds of MB of pinned memory, permanently, per worker), trailing bytes are rejected, and the stream is synchronised before any throw that happens after work is queued. An image of fewer than 8 elements is all verbatim tail and now decodes rather than throwing. When the device route fails for any reason the host decoder gets its turn, so it costs speed rather than the acquisition. The lanes cooperate on the copies and a later match can read bytes another lane wrote, which since Volta needs an explicit __syncwarp(); it worked only because ptxas happened to reconverge at the post-dominator. The prototype's offset == 1 and power-of-two fast paths are also restored - the shipped kernel ran a runtime modulo, an emulated 32-bit division per output byte, on the path its own comment calls the common case. The un-transpose is now fused with preprocessing. One thread owns one group of 8 elements across every byte-plane, so once it has transposed its 8 bytes out of each plane it holds 8 complete elements and emits 8 finished int32 pixels with the mask, the error marker, the saturation cap and the statistics applied. The decompressed image is never materialised: 0.623 -> 0.411 ms/frame at 18 Mpx, 0.523 -> 0.340 with 8 concurrent workers. Staging nothing in shared memory also drops the 48 kB ceiling, which had made any file whose bitshuffle blocks exceed it a hard failure; 64 kB blocks now decode. gpu_compressed is sized from the chunk with grow-on-demand instead of from the uncompressed size - it was reserving ~73 MB per worker to hold ~4 MB. Measured on a 1630x1553 uint32 rotation set at -N 32, peak GPU memory falls 3756 -> 3084 MiB; the same model gives ~144 MB per worker on an 18 Mpx frame. Decoding on the device also stopped reporting a decompression time, which blanked the broker's compression plot trace and filled /entry/profiling/compressionTime with NaN. The decoder brackets the decode with CUDA events and reports it again. Tests: a differential fuzz suite against the CPU decoder - incompressible and highly compressible data, engineered offsets, a size sweep hitting every rem%8 value twice, all six element sizes, an 18 Mpx frame, decoder reuse, concurrency, hand-built LZ4 blocks across the overlap boundary, 26 foreign bitshuffle block sizes from 128 B to 64 kB, corrupt payloads and malformed containers, with a coverage report that proves which LZ4 paths were reached rather than assuming it. Plus the fused path held byte for byte against ImagePreprocessorCPU, statistics included, and against the host-upload path on the same frame. Battery: 37 crystals, every merged number identical to the host-decode run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:13:55 +02:00
leonarski_fandClaude Opus 5	47277674fa	docs: split the breaking API changes out of the rc.161 change list ... Earlier releases put breaking changes in their own paragraph after the bullets (rc.139, rc.29) rather than as one item among them. Follow that: the OpenAPI changes now sit under their own heading below the list, with the client-side action in the lead line, and are listed one per change instead of run together. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 07:46:57 +02:00
leonarski_fandClaude Opus 5	737cbde3ff	docs: tighten the changelog and the decoding section for release ... The rc.161 changelog had grown entries several hundred words long and listed the same area three or four times over. Collapse them by subject - spot finding, resolution limits, space-group search, scaling, performance, correctness - and hold each to one line, keeping the actionable detail in the breaking API entry. Add the performance work that had not been written up: device-side image decoding and the parallel first-pass candidate-cell refinement. Section 0 of the CPU analysis document was the longest thing in it after two core algorithm sections, and most of that was a profiling narrative and the measurements that motivated the change rather than a description of what runs. Cut it to the two kernels, the host-side block scan and the fallback rule. The attribution stays; it is also in the reference list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 07:44:55 +02:00
leonarski_fandClaude Opus 5	7e47afe47f	rugnux: parallelise candidate-cell refinement, and stop repeating work in the tail ... Three independent changes to the CPU-bound parts of an offline rotation run, none of which alters a result. Candidate-cell refinement now splits across threads. RefineCandidateCells already took a (block, nblocks) partition, but the only call site passed nblocks=1, so the whole first pass of a two-pass rotation run sat on one thread per scheme - two threads, unchanged at every -N, for a third of the run. A block touches only its own scores(j) and cells rows and holds its own scratch, so the split is exact. The budget is a new IndexingSettings::RefineThreads, left at 1 by default and set only where few indexer threads exist: raising it unconditionally would oversubscribe the paths that already run one indexer per image across all workers. The mmCIF writer built a std::ostringstream per formatted number, twelve per reflection. snprintf gives the same digits for 0.535 -> 0.220 s per file. The space-group search built the same orbit mapping twice per candidate point group - once for the merge chi^2 and once for the systematic-error b, an apply_to_hkl and Canonicalize per observation per operator each time. Build it once and hand it to both. 18 Mpx rotation set 24.6 -> 18.7 s, 2.5 Mpx 13.0 -> 10.7 s, and the 37-crystal battery 13m55s -> 10m47s with no failures, the same 34/37 space groups, and statistics unchanged on 30 of 37 (the rest drift within the run-to-run spread the binary already had, which a control build with the split disabled reproduces). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 07:30:33 +02:00
leonarski_fandClaude Opus 5	13aa20a528	bragg_integration: grow the GPU reflection arrays with slack ... EnsureCapacity resized its 13 device arrays to exactly the current image's predicted-reflection count, so every image that set a new record freed and reallocated all of them. cudaMalloc and cudaFree take a device-wide lock in the CUDA driver, so those images stalled every other worker: sampling the worker threads during the per-image loop found 21-24 of 32 parked in cuMemAlloc_v2 or cuMemFree_v2, all called from this one function, and the running maximum makes 32 workers do far more allocator work than one does. Grow by half again instead. All transfers and kernel launches are sized by the per-image reflection count rather than by the capacity, and the member is already documented as holding at least that many, so over-allocating changes no result. On an 18 Mpx rotation set the integration stage drops from 1.37 to 1.25 ms per image at 32 workers; merged statistics, error model and adopted space group are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 01:02:07 +02:00
leonarski_fandClaude Opus 5	6e4c0ce202	image_preprocessing: decode bitshuffle+LZ4 on the GPU ... The pipeline decompressed each image on the host and uploaded the result. On an 18 Mpx rotation dataset that made the host-to-device copy the bottleneck of the whole per-image loop: nsys puts the copies at 78% of the loop against 39% for every kernel combined - 3600 transfers of 72.4 MB - and they ran at only 12.5 GB/s of an available 27-28 because the host-side decompression was itself saturating host memory bandwidth. The GPU was mostly waiting. So the compressed chunk goes across instead, about 4 MB rather than 72 MB, and is decoded on the device. That removes the transfer and the host decompression that was throttling it, in one change. Measured on an idle machine, a run goes from 45.11 s to 24.97 s - 1.81x - with the merged output unchanged. THE APPROACH IS JON WRIGHT'S (ESRF): "Experiences with GPU decompression for bitshuffle + LZ4 data", HDF5 User Group 2021, and github.com/jonwright/ bslz4decoders. The kernels here are ours, but the idea and the demonstration that it is worth doing are his. Cited in docs/ACKNOWLEDGEMENT.md and in the new section 0 of docs/CPU_DATA_ANALYSIS.md. Two kernels mirror the CPU decoder. LZ4 runs one WARP per bitshuffle block: every lane parses the same sequence stream (a broadcast read, no divergence) and the literal and match copies are split across the 32 lanes so the stores coalesce; an overlapping match is treated as a pattern of period offset sourced from bytes that already precede the write position, which keeps it parallel rather than a serial byte loop. One thread per block instead measured 13x slower. The bitshuffle inverse then un-transposes each byte-plane through shared memory and interleaves the planes back into elements. Only BSHUF_LZ4 is decoded on the device. The zstd variants have no device decoder, and neither has an uncompressed or float image; Supports() returns false for those and the caller decompresses on the host exactly as before. The fallback is explicit, so a format we cannot decode on the device is a slower path and never a wrong answer. Tests hold the device decoder against the CPU one byte for byte, on data from the production compressor, for every element size the detectors emit - including the 8-bit DECTRIS modes, which take bitshuf_decode_block's separate elem_size == 1 branch - plus a many-block frame, the formats it must decline, and malformed containers, which must throw rather than run off a buffer. Battery: 37 crystals, no failures, identical to the host-decode run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 00:16:36 +02:00
leonarski_fandClaude Opus 5	59702b0123	spot_finding: give the ring reduction eight blocks per SM instead of four ... reduce_rings_shared is the largest kernel in the per-image loop - 73% of GPU kernel time on an 18 Mpx rotation run, launched three times per image - and it is bound by shared-memory atomic replay rather than by bandwidth: it reaches 156 GB/s against a measured 913 GB/s ceiling, and removing the atomics while keeping the same loads makes it five times faster. That is the case that wants resident warps to hide the serialisation, and four blocks per SM left only 512 of the 1536 threads an SM can hold. The per-block histogram is nbins * 20 B, about 9.6 kB at the default 0.01 1/A spacing, so eight blocks fit in shared memory with room to spare. Both kernels are grid-stride loops, so any grid is correct and a device that cannot co-schedule eight simply queues the rest. Measured: 9.21 s -> 5.33 s of kernel time over a run (852 -> 493 us per launch), cutting total kernel time from 12.57 s to about 8.85 s. flag_strong keeps four. It is bandwidth-shaped rather than atomic-bound and eight measured no better (181 vs 175 us). Wall clock is unchanged, and that is expected rather than disappointing: kernels are 39% of the image loop while the host-to-device copy is 78%, so faster kernels idle the GPU more without shortening the loop. This is groundwork for the transfer work, not a speedup on its own. The shared accumulators are float and summed with atomics, so the block count changes the summation order and with it the last bits. The 37-crystal battery is identical crystal for crystal except one observation in 925850 on a single dataset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 00:02:34 +02:00
leonarski_fandClaude Opus 5	a047275760	spot_finding: fix the GPU finder's main loop, which the tests could not reach ... Two bugs in analyze_pixel, both confined to the middle stage of the wave. The kernel walks each wave's rows in three stages. The priming and drain loops read prev_out and substitute INT32_MAX for a pixel the previous pass found strong, exactly as the CPU finder's value_at() does on every read. The main loop did not - it read the image raw. So in the second pass the pixels the first pass found strong stayed in the background statistics, inflating the local mean and variance, and the halo of every broad spot failed the signal-to-noise test. The two engines therefore did not agree, despite 1a1e05ad1 having set out to make them. Separately, shared_sum2 is an int64 accumulator but val*val and old*old were computed in int32 at three sites. That wraps above 46340 while the detector overloads around 1e6, so any window containing a bright pixel got a corrupted variance. pixel_result already did the same arithmetic in 64 bits. The reason this survived is worth recording: numberOfWaves is fixed at 32, so a wave owns ceil(height/32) rows, and the main loop only runs while front < rmax with front starting NBX+1 = 16 rows ahead. At the existing test's 100 rows a wave owns 4 rows and THE MAIN LOOP NEVER EXECUTES - every row goes through priming or drain, and the parity test passed on the broken kernel. On a 4362-row detector frame a wave owns 137 rows and the main loop carries about 121 of them, so the bug covered roughly 88% of a real image. The new tall-image test is sized to the partitioning rather than to convenience: 1024 rows, spots placed inside the main-loop region, one core at 100000 to exercise the overflow. Against the unfixed kernel it reports GPU 25 pixels where the CPU finds 49 and fails six assertions. The default rugnux path is unaffected because it uses the adaptive finder, and the 37-crystal battery is identical crystal for crystal. The classic finder is what SpotFindingSettings defaults to, so this is the online receiver's path; measured there with --no-adaptive-spots, R-meas improves 11.2% -> 10.9% and CC1/2 98.8% -> 98.9% on one crystal, with multiplicity up on both tried. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 23:31:44 +02:00
leonarski_fandClaude Opus 5	83e95b0c5a	indexing: stop computing angles the candidate filter only compares ... Candidate cell filtering called acos three times per candidate to turn dot products into degrees, then compared those against the min/max angle bounds. acos is strictly decreasing on [-1, 1], so "angle outside [min, max]" is exactly "cosine outside [cos(max), cos(min)]" with the ends swapped - the bounds convert once, and the three acos calls per candidate disappear. The same loop also re-derived every already-accepted candidate's unit cell on each new triple, inside the duplicate scan: three more acos each, for every candidate accepted so far. Those cells are now kept alongside the candidates. Measured on de-novo serial stills, where the indexer runs once per image: 34.43 s -> 14.17 s on one dataset and 21.92 s -> 6.59 s on another, with the indexing rate and the merged reflection count unchanged (one gained 0.25 points of indexing rate). acos had been 40% of the whole process there. Scope is narrower than that number suggests, and worth stating: the win is on the de-novo path, which Auto selects for stills only when NO cell is known. With a known cell Auto picks ffbidx, which reaches the same filter but feeds it few candidates - measured neutral there (+0.5% instructions, -1.6% wall, identical output), and that path already runs 14x faster in absolute terms. Rotation runs the indexer twice per dataset rather than per image, so it is unaffected: the full 37-crystal battery is identical, crystal for crystal. Comparing cosines instead of angles can only move a candidate that sits on the bound, so the filter's behaviour is unchanged except at that measure-zero boundary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 21:24:57 +02:00
leonarski_fandClaude Opus 5	87f31fa06c	ci: record why LTO is a flag and not CMAKE_INTERPROCEDURAL_OPTIMIZATION ... The CMake variable is the tidier spelling and would cover the MSVC job too, so it is the obvious thing to reach for and worth saying why it was not. It builds and links, CUDA included - and it does not reach .cu targets either way, so there is no -dlto risk on either route. But it optimises less: 396.9 G retired instructions against 384.9 G for -flto=auto, three runs each, with a 0.45% run-to-run spread, so a 3% gap is not measurement luck. Of 107 static libraries the two routes agree within 5% on 105; the flag additionally covers FFTW and libzmq. And CMAKE_AR stayed plain ar under the variable, so the archive-handling argument for it did not hold here either. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 20:32:14 +02:00
leonarski_fandClaude Opus 5	d55f3257ed	ci: regenerate the published API artefacts, and build Linux with LTO ... update_version.sh had not been run for the adaptive spot-finding schema change. Running it leaves the C++ server model and the TypeScript client byte-identical to what the generators produced directly, but it also regenerates two artefacts the direct calls do not touch and which are tracked: the Python client's published documentation and the Redoc bundle. Both now carry adaptive_threshold and false_pixels_per_frame. LTO joins -march in the CI flags, which is why MARCH_CMAKE_FLAGS is now LINUX_CMAKE_FLAGS - it no longer describes only the architecture. Measured on rugnux against an otherwise identical build: 7-10% fewer retired instructions and a 9% smaller binary, but only ~1.5% off the wall clock, because the pipeline is GPU- and I/O-bound. It costs about 3x on an incremental rebuild (9.8 s -> 30.1 s for one file plus link), so it stays out of CMakeLists and out of a developer's edit cycle: CI builds from scratch and ships the result, paying the link once. It links against CUDA with no special handling, and both CI images already put gcc-toolset-13 on PATH, which -flto=auto requires. MSVC is left alone: its LTO is a different flag (/GL + /LTCG) and nothing here measured it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 20:21:50 +02:00
leonarski_fandClaude Opus 5	90ab67f376	broker: expose adaptive spot finding over the API ... The self-calibrating detection threshold was reachable from rugnux and the viewer but not online: spot_finding_settings carried no adaptive_threshold, so the receiver always ran the fixed-threshold finder and the fused GPU engine sat unused behind it. adaptive_threshold and false_pixels_per_frame are now part of the schema, both optional so an existing client that sends neither is unaffected, wired through OpenAPIConvert in both directions and surfaced in the frontend panel, where turning the mode on greys out the count threshold it replaces and reveals the operating point it uses instead. The C++ server model and the TypeScript client are regenerated from the spec; the Python client is generated but not tracked. Enabling it is REFUSED where spots are found on the FPGA - the JUNGFRAU and EIGER workflows - rather than accepted and ignored, because a detection setting that silently had no effect cannot be told apart from one that did. The DECTRIS/SIMPLON workflow, which analyses images in software, accepts it. Verified against a running broker: adaptive_threshold true is rejected with that message and leaves the stored settings untouched, while false and omitted both succeed. It stays off by default online, unlike rugnux and the viewer. The broker serves both workflows and the default has to be the one that works on either. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 20:01:34 +02:00
leonarski_fandClaude Opus 5	c4f5e62e86	tests: lock the whole reflection round trip, not six fields of it ... The round-trip test checked h, k, l, I and the two predicted coordinates. The other ten fields were written and read back unasserted, which is how the offline --scale path came to lose image_number without a test noticing - it is the field 3D-integrated reflections carry a fractional value in, and rocking events cannot be grouped without it. Reflections are now built by a helper that puts a distinct value in every field that is meant to survive, keyed on the image and the reflection index so a value read back from the wrong place cannot match, and checked by one that asserts all of them. Verified by reintroducing the image_number loss, which fails six assertions and passes none of them silently. dist_ewald, observed and on_ice_ring are deliberately excluded and the test says why: the first two are prediction/integration scratch that is never written, and the third is recomputed from the resolution by whoever scales. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 19:52:46 +02:00
leonarski_fandClaude Opus 5	ea667cb306	rugnux: handle ice rings in --scale as the full pipeline does ... --scale did none of the ice handling the run that wrote the _process.h5 had done, so re-scaling a stored dataset silently produced a different - and flatteringly more complete - answer than the pipeline it was meant to reproduce. Three separate gaps: * --detect-ice-rings was accepted and ignored. The --scale block returns before the line that applies it. * Reflections were never flagged as sitting on an ice ring, so the per-image scale fit included them. The flag is not stored per reflection, so it has to be recomputed from the resolution. * RotationScaleMerge was constructed with the ice half-width hardcoded to zero. That is what turns a resolution into a ring index, so every ice test inside the merge was a no-op whatever was passed to it. The CC1/2 ring test that decides which rings to drop moves into FindDecorrelatedIceRings, shared with the full pipeline so both reach the same verdict on the same data, and --scale now re-merges with the mask the way the pipeline does. The stills branch re-runs only the merge: the scaling has already been applied to the reflections and repeating it would compound it. Measured on a rotation dataset with three decorrelated rings, --scale went from 8765 unique / 36.3% completeness / R-meas 18.5% / <I/sig> 1.1 to 7638 / 31.6% / 18.0% / 1.3, against the full pipeline's 7692 / 31.8% / 17.9% / 1.3 - the reported completeness had been inflated by reflections the pipeline drops. The full pipeline is bit-identical across the refactor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 19:49:55 +02:00
leonarski_fandClaude Opus 5	5f30f0d1ac	tests: hold the integer coordinate conversion to the module map ... ConvertToImageCoordinates now transforms the photon-weighted sums instead of the centroid, which is only equivalent because a module's raw -> image map is a signed axis swap plus an integer translation. Check that against the map itself on every module of a detector whose modules do not share an orientation, and either side of the 256-column multipixel gaps where the translation changes. A wrong sign or a dropped gap term on any single module would otherwise only show up as mispositioned spots on that module. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 19:14:32 +02:00
leonarski_fandClaude Opus 5	b6d3dcc6fe	rotation_indexer: demand a decisive margin before adopting an axis multiple ... Candidate selection promoted a later cell whenever it indexed 0.05 more of the accumulated spots. That margin is not meaningful when the candidate is a near-integer volume multiple of the incumbent: multiplying an axis halves the reciprocal spacing, so the multiple has a lattice point wherever its sub-cell has one and another in between, and it collects spots the sub-cell leaves unindexed for reasons that have nothing to do with the crystal. The indexed fraction is biased in its favour, and a small lead is not evidence. On one rotation dataset the true cell and a spurious 5x supercell were separated by 0.003 of indexed fraction against a bar of 0.05 - close enough that the -march flags the binary happened to be built with decided it. The baseline build kept the true cell and merged to an R-free of 0.24 against an external model; an -march=x86-64-v3 build (what CI uses) took the supercell, carried it into a doubled cell and a different space group, and merged to an R-free of 0.58, which is noise. Both were reproducible, five runs each, and independent of thread count. An integer multiple now has to index 1.5x the incumbent, the same shape the lower-symmetry-setting guard next to it already uses. A real superstructure's satellite rows are a large share of its spots and clear that comfortably. Both builds now agree on the true cell with a wide margin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 19:03:47 +02:00
leonarski_fandClaude Opus 5	8f1b0b2281	spot_finding: accumulate spot centroids in integers ... The photon-weighted position sums were floats, so the centroid's last bit depended on the build rather than on the data: gcc contracts the multiply-add in AddPixel into an FMA under -march=x86-64-v3 and cannot at the baseline, and MSVC does not contract at all under /fp:precise. The GPU extractor had to match with __fmaf_rn, and the parity test still needed a two-ulp slack for hosts that do not fuse. Column, line and the per-pixel count are all integral, so the sums are exact in int64 and both implementations reach the same bits with nothing to match. The parity test now demands exact equality unconditionally and gets it, including on a baseline build. ConvertToImageCoordinates keeps the sums integral too: the raw -> image map is a signed axis swap plus an integer translation, so it is applied to the sums instead of to the centroid. Drops the SpotToSave constructor, which had no callers and could not have been converted without quantising the stored centroid. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 19:03:35 +02:00
leonarski_fandClaude Opus 5	cd16053c2b	rugnux: use the project PI constant, not M_PI ... MSVC does not define M_PI without _USE_MATH_DEFINES, and rugnux is part of the portable subset that JFJOCH_VIEWER_ONLY builds. JFJochMath.h already carries PI for exactly this reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 18:53:06 +02:00
leonarski_fandClaude Opus 5	e7be5447d3	receiver: stop copying every frame back from the device on the Lite path ... The Lite workflow built its analysis with the fused GPU engine disabled, which is also what decides whether the preprocessed image is copied device-to-host after every frame. So on a machine with a GPU the online path was moving the whole image back - 72 MB on a large detector, every frame, per worker - for a host reader that does not exist on that path. It was left off deliberately when the fused engine was added, to keep the online path unchanged in that commit, and never revisited. Nothing depends on it: the FPGA workflow uses a different analysis class, and strong-pixel values are read through a device gather rather than from the host image. Turning it on changes no result, and cannot: adaptive detection is unreachable online, because the REST schema exposes no way to enable it, so the classic GPU finder runs either way. Measured anyway, both engines on the same frames across five datasets including very weak ones: 2400 frames, 638260 spots, not one difference - identical lists, identical indexing rate, identical merge statistics to every printed digit. On a large detector with eight workers the median per-image cost falls from 94 to 59 ms and preprocessing from 21 to 6 ms; throughput rises from about 48 to 55 Hz. No percentile regresses, which is what matters for a service - the ninetieth improves from 128 to 74 ms and the tail with it. Spot finding gets faster too, because the large copy no longer contends with the device gather. Correct two statements while here. The flag's comment and the data-analysis document both said the online receiver uses the CPU adaptive finder; online never runs an adaptive finder at all, and the copy the flag really controls was not mentioned. That copy would be better expressed as what it is - whether a host engine will read the image, which the constructor already knows - rather than inferred from which spot finder is wanted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 15:16:18 +02:00
leonarski_fandClaude Opus 5	ccbc366e2f	reader: read the frame number back with the reflections ... The per-reflection frame number is written to the process file and always has been, but the reader's designated initialiser simply omitted it, so every reflection came back at frame zero. Nothing complained, because zero is a valid frame. It matters because the 3D combine splits a reflection's partials into rocking events by frame contiguity. With every observation claiming frame zero there are no gaps to split on, so a reflection's entire rotation range collapses into ONE event: measured on a rotation dataset, 216066 fulls against 216705 distinct reflections, where the pipeline finds 367416. Forty-two per cent of the observations disappear, the goniometer-frame absorption surface evaluates every observation at a single angle, and the radiation-damage estimate is computed over a run that appears to last no time at all. The reason this survived is that the damage flatters: fewer, better-agreeing observations per reflection give R_meas sixteen per cent lower, ISa thirty per cent higher and a slightly better CC1/2 than the real merge. Anyone re-scaling a stored file was reading numbers that looked better than the pipeline's while standing on less than two thirds of the data, and one radiation-damage figure that was pure artefact. Read it as mandatory rather than optional-with-default, like h/k/l and the intensities: a silent zero is precisely the failure being fixed, and every file this function can read carries the dataset. After the fix the combine reproduces the pipeline exactly. The normal path does not go through this reader and is byte-identical before and after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 13:37:13 +02:00
leonarski_fandClaude Opus 5	4bdb229fb8	spot_finding: find connected components on the GPU ... The spot finder flagged strong pixels on the device and then labelled them on the host, so every frame sent the packed bitmask back - 2.26 MB on a large detector - and the host walked all of it to recover a few hundred pixels. Do the labelling on the device instead: compact the bitmask into a flat-index-sorted list, find each pixel's backward neighbours by binary search, union them lock-free with path halving, then label, accumulate and filter in one kernel. Only the spot list comes back, and only one stream synchronisation per frame. The gain in the ordinary case is modest - about a quarter off per-image spot finding - because the host algorithm is genuinely fast on a normal frame. What justifies it is the frame that is not ordinary. The host labels a sorted sparse list through a window spanning two detector lines, so its cost is quadratic in how many strong pixels share a line. A lit band of detector rows - a hot module, a panel edge - costs 33 ms at two rows and 377 ms at fifteen, all of it under the pixel cap that was supposed to bound this, and none of it maskable when the cause is a diffraction ring rather than a defect: a ring runs tangent to a row at its top and bottom, which is exactly the shape that hurts. The device version is flat at 0.05 to 0.64 ms across every geometry tried, so an online run no longer stalls a quarter of a second on an ice ring. Rejecting an over-cap frame is now free too, since the count is known before any pixel is written. Also label once and filter three times. The per-image minimum-pixel search runs the extraction at three settings, but that setting only decides which components are kept - it does not change the components - so the search itself need not be repeated. This helps the host path as much as the device one. The resolution mask moves to the device as a bit mask, uploaded when the limits change rather than per frame, since the compaction needs it there. Parity is asserted permanently rather than argued: five cases covering realistic frames, occupancy from a hundred pixels to past the cap, the pathological geometries including rings, the resolution mask, and a hundred-repeat determinism check - requiring the same partition, the same spot order, and identical counts. The centroid is a float sum and therefore order-dependent, so the device walks each component from its root in ascending order and fuses its multiply-add the way the host's does; note that whether the host fuses at all depends on the architecture flags, so exact centroid equality is asserted where the compiler fuses and a two-ulp bound otherwise. Making those accumulators integer would remove that dependence entirely and is worth doing separately. Regression set: all 37 crystals identical to the last printed digit. Unit suite passes with the new cases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 13:34:43 +02:00
leonarski_fandClaude Opus 5	e4d70f0e55	image_preprocessing: inline the buffer accessors ... operator[], size(), data() and getBuffer() are one-line accessors that were defined in the .cpp. The build sets no link-time optimisation, so out of line each of them is a real call - once per pixel, from the CPU preprocessor, the CPU azimuthal integrator and the CPU spot finder - and they stop those loops vectorising at all. They show up in a profile directly: about six per cent of a whole azimuthal-integration-only run is spent in the call overhead of two accessors that do nothing but index a vector. Moving them into the header retires 30% fewer instructions on that run and takes the per-image CPU cost on a GPU-less pass from 34.6 to 24.2 ms, with the output bit for bit unchanged - same observation count, same cell, same merge statistics. It is worth nothing on the GPU path, where the image stays on the device, and everything on the paths that have no GPU to fall back on. This also explains a measurement that had been blamed on the pixel mask being a vector<bool>: a microbenchmark of that loop indexed a raw pointer and came out far faster than the same loop in the binary, and the difference was this call, not the mask. Measured properly the mask costs about 14% single-threaded rather than the 41% claimed, and at the thread counts this actually runs at the bit mask is FASTER than the byte mask it was proposed to become, because it moves eight times less traffic and the loop is bandwidth bound. That change should not be made. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 11:20:31 +02:00
leonarski_fandClaude Opus 5	639fbb3fbc	indexing: select predicted reflections by partiality, build indexers where it pays ... When more reflections are predicted for a frame than the output can hold, the surplus was dropped by keeping those closest to the Ewald sphere. On the rotation path that quantity is identically zero by construction - the rocking coordinate is chosen so the scattering vector lands exactly on the sphere - so the comparison fell through to h, k and l and the survivors were whichever came first in lexicographic order. Measured on a large cell: every value within one float ulp of zero, and the kept set had a MEAN PARTIALITY BELOW that of the full set, i.e. worse than choosing at random. Rank by partiality instead, which the predictor already computes and which is what the header always claimed was being kept. On the one regression crystal large enough to cross the cap this lifts completeness from 84.8% to 90.2% on the same observations; multiplicity and R_meas move the way they must when the same measurements cover more of reciprocal space. The online path asked for a cap of ten thousand but the truncation was hardcoded to the offline limit, so the broker predicted and integrated up to six times what it could transport and discarded the rest after paying for it. Honour the caller's limit, which also makes the post-integration re-truncation dead code. Indexer pool construction becomes a policy. The online service needs every indexer resident before data arrives, because a cuFFT plan built on the first frame is planning time inside the measurement; spending memory to be ready is the intended trade there and stays the default. Offline there is no such deadline, and a stills run with a known cell was holding a fully allocated FFT indexer per worker that the algorithm resolution can never dispatch - 2.8 GB where 0.4 GB is needed. rugnux and the viewer opt into building on first use; the broker, the receiver and the tests are untouched. This also removes a dangling reference that was latent: the worker held the settings by reference although the pool is routinely constructed from a temporary, which only survived because eager construction finished inside the constructor call. Finally, refuse a first-pass lattice that indexes fewer than a sixth of the validation frames. It fires on nothing in the regression set - the weakest real crystal sits at 22 of 60, more than twice the floor - so it is a backstop, but the failure it prevents is one the set does contain: a dataset with no crystal at all adopts a lattice from its powder rings, integrates every image against it, and dies much later inside the merge complaining about resolution. It now stops in the first pass and says what to try. Regression set: 36 of 37 crystals byte-identical, the exception being the completeness gain above; 34 of 37 space groups, no failures. Full unit suite passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 09:12:27 +02:00
leonarski_fandClaude Opus 5	b47bce7c3b	ci: give the MSVC viewer /arch:AVX, and write down why -march lives in CI ... The Linux jobs already pass -march=x86-64-v3; the MSVC viewer job passed nothing, so it built at the x64 baseline. MSVC has no spelling for the x86-64-v2 level, but /arch:AVX is the nearest and implies SSE4.1/4.2 - which is the part that matters, because below SSE4.1 Eigen has no vectorised round and falls back to one libm call per element. AVX is Sandy Bridge and up, a safe floor for a desktop viewer. The architecture flags stay OUT of CMakeLists on purpose, so a site can build x86-64-v4 on an AVX-512 cluster, or -march=native, or the plain baseline. That is easy to mistake for an oversight and "fix", so say it in CLAUDE.md - together with the consequence that catches anyone profiling: a default local Release build is not what CI or production runs, and the gap is not uniform. GPU-bound work is unaffected, but the CPU and Eigen bound phases - first-pass indexing and scaling/merging - measure about 26% slower without the flags. That is enough to make rounding look like a tenth of all cycles when a real build has it nearly free, and to send a reader at the wrong code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-01 21:58:36 +02:00
leonarski_fandClaude Opus 5	6e805f53c0	image_analysis: stop paying for work that is thrown away ... Three independent costs, each measured, none changing a result. Across the 37-crystal regression set the run time halves (median per crystal 2.0x, total 2.3x) and every crystal's merge statistics are unchanged. The image copy back from the device moved the whole preprocessed frame - 72 MB on a large detector, every frame, per worker - to serve a single host consumer that reads only the strong pixels, at most a few hundred kilobytes of it. Give the buffer a Gather() so that consumer asks for the values it actually wants (a host loop on the CPU, a small kernel on the GPU), and copy the frame back only when a CPU spot finder will genuinely read it. The copy the other way was worse: it came from an unregistered vector, so the driver staged it through its own pinned pool with a host-side memcpy on the calling thread, which does not overlap and collapses under concurrency - 11.6 GB/s at one worker, 1.6 GB/s at eight. That, not any hardware limit, is why throughput stopped improving past four to eight workers. Pinning the decompression buffer once per worker fixes it: on a 18 Mpx dataset the image loop goes from 13.6 to 7.9 ms per image at 32 workers, and 32 workers now beat 8 instead of losing to them. Ceres was computing seventeen partial derivatives where five are free. The per-image rotation refinement frees the beam and the orientation and holds distance, detector angles, rotation axis and cell constant, but the cost function declared all seven blocks, so every residual evaluated in Jet<17> arithmetic. A residual exposing only the two free blocks - the same arithmetic, the constants baked in - halves refinement, and it is exact rather than merely close: dual coordinates evolve independently, so the residuals and the free Jacobian columns are unchanged bit for bit. The merge sorted an index array with a comparator that dereferenced a 1.6 GB array of 72-byte records, i.e. a random walk over memory, single-threaded, twice per two-pass run. Sorting a packed key instead is 2.4x. French-Wilson allocated its integration scratch per reflection and ran serially; it now takes caller-owned scratch and runs over chunks, 4.2x. The correction surfaces re-tested every observation for usability and parity on each of ~22 passes and re-allocated their accumulators each time; bucket the indices once and hoist the buffers. Also convert std::round to std::rint where the rounded value only ever enters a squared residual. The tie rules differ - away from zero against to even - so this is safe exactly where a tie flips the sign but not the magnitude, and unsafe wherever the value becomes a Miller index; those sites keep std::round. Verified over all 2^32 float bit patterns: 8388608 exact ties exist, and the squared residual is bitwise equal for every one of them. Worth little on its own here, because the rounding that dominates is in candidate refinement, where the value is an index and the substitution is not available. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-01 21:35:28 +02:00
leonarski_fandClaude Opus 5	bfb8cb813c	rugnux: report per-image cost honestly instead of per-worker blocked time ... Each stage timer measures wall time inside one worker, so it counts whatever that worker spent blocked on a contended resource - above all the single GPU - as well as its own work. Those waits overlap across workers, so the mean was printed as if it were the per-image cost when it is roughly the per-image cost times the worker count. At the default thread count on a large detector the reported total came out more than twenty times the truth, and single stages were printed as several times the entire per-image budget of the run. That is the one output anyone tuning performance reads, and it sent this investigation at the wrong stage for a while. Divide by the worker count. It is a lower bound - a worker idle rather than blocked is not counted - so rather than hide the remainder, report the image loop's own wall time next to it, and with it the time spent OUTSIDE the loop. Nothing measured the latter before, yet on a rotation run the first-pass indexing and the scaling and merging can be more of the run than the per-image work is: on a large-detector run here it is 5.1 s against 3.0 s. Both figures are for the last pass, and a two-pass rotation run does all of it twice. Also stop printing nan. The per-image indexing and scaling timers are never fed on the two-pass rotation path, because the lattice is forced rather than searched per image and the merge happens outside the loop, so every default rotation run reported "indexing nan scaling nan". A stage that did not run is now simply absent. Measured against the loop's own wall clock on a 18 Mpx dataset: 5% at one worker, 11% at eight, 29% at thirty-two, versus 23x too high before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-01 14:49:45 +02:00
leonarski_fandClaude Opus 5	0ae1a307bc	indexing: complete a rank-deficient direction set, and keep the higher-symmetry setting ... The FFT shortlist could be rank-deficient, and then no cell could be formed at all. FilterFFTResults takes the strongest max_vectors RAW directions and only then prunes ones within 5 degrees of each other, but a single lattice row is sampled by many neighbouring directions of the 16k half-sphere, so thirty raw peaks routinely prune down to four or five distinct directions - the strongest, hence shortest, rows. When a crystal's densest rows share a plane, every surviving direction is coplanar, every triple the reduction forms is degenerate, and the indexer returns nothing. On such a crystal the weak third axis was the eighth distinct direction, at raw rank 78. Keep walking the same magnitude order for up to four more directions that are 5 degrees clear of everything kept, appended after the length sort so the earlier entries hold their positions and the reduction still forms every triple it formed before - the shortlist only gains candidates at its end. That exposed two ways a change of SETTING was mistaken for a different lattice. A centred conventional cell is an exact integer multiple of its primitive one, so the same lattice described two ways differs by that factor: comparing conventional volumes reads a setting change as a sub-cell or a supercell. Both the candidate selection in the rotation indexer and the pass-2 comparison in the driver did exactly that, and between them they discarded a correctly-classified cubic F cell in favour of the body-centred tetragonal description of the very same lattice. Compare primitive volumes in both, as the scheme comparison already did. Fixing the volumes alone was not enough, because the indexed fraction is also biased across crystal systems: a subgroup setting holds fewer cell parameters fixed than its supergroup, so it can never index fewer spots and will always look better by that measure. Where a candidate has a lower lattice point-group order at the same primitive volume - the signature of the same lattice in less symmetry - require it to index markedly better, not merely better, before it displaces the incumbent. A general metric-symmetry promotion was implemented and rejected on evidence: it raised a correct body-centred orthorhombic cell to triclinic and a monoclinic one to C-centred orthorhombic, and no threshold separates the cases, because a false pseudo-orthorhombic degeneracy measured tighter than a true cubic one on obliquity and on alternative-basis axis excess alike. Metric alone cannot decide this; only the intensities can, which is what the space-group search is for. Measured over the 37-crystal regression set: one crystal goes from failing outright to 91% indexed with 91% completeness and a better R_meas than the reference, one keeps the cubic setting it had before, and every other crystal is byte-identical. Full unit suite passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-01 12:59:34 +02:00
leonarski_fandClaude Opus 5	2238290d6d	rugnux: make spot settings reach rotation indexing, and stop over-claiming ... Three small honesty and cost fixes on the two-pass rotation path. Spot-finding settings did not reach the step that determines the unit cell. The two-pass first pass reuses the spots stored in the file whenever it has them, and reuse is the default, so both sampling schemes and the validation loop ran on acquisition-time spots while only the per-image pass saw the command line. Every --spot-* option was therefore a no-op for the lattice search on any file written by this software, silently, and the lattice was cross-validated against one spot set and applied to another. Giving spot settings now implies re-finding them for the first pass as well, and plain reuse says so in the log. The summary printed a space group and unit cell even when nothing indexed. With a zero indexing rate the cell is whatever the lattice search happened to return, no reflection was ever measured on it, and no output file is written - so stating it as the run's answer claims a result the data do not support. Say that no lattice was determined instead. The second pass re-indexes de novo so the cell comes out self-consistent with the post-refined geometry, and its result was already checked against the first pass - once by the supercell test and once by the centring test - but only after every image had been integrated with it, so a disagreement cost a whole extra pass on a dataset that ended up on the first pass's lattice regardless. Compare them at the point the lattice is adopted instead, using the same two tests and the same fallback. A triclinic de-novo cell is left alone, being the demotion the merge reindexes. Measured over the 37-crystal regression set: merge statistics are unchanged on every crystal (the two that move are the known rotation-indexing non-determinism - one observation in 2.9 million, and a zero-score lattice landing on no partials instead of a few). Crystals whose data were already cached in the reference run are unchanged in wall time. The one dataset that was burning a discarded pass went from three passes to two, 509 s to 198 s, against 0.81x for the same-detector dataset that was already running two passes - so about 214 s of the saving is the removed pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 23:13:11 +02:00
leonarski_fandClaude Opus 5	0b1fb6c870	image_analysis: share the read-only GPU lookup tables per device ... One analysis engine is built per worker thread, and each uploaded its own copy of tables that are pure functions of the detector geometry: the pixel -> azimuthal bin map and the per-pixel corrections (both in AzIntEngineGPU AND again in AdaptiveSpotFinderGPU, from the same mapping), plus the pixel mask. On an 18 Mpx detector that is ~224 MB per worker; with 32 workers ~7 GB of device memory held 32 identical copies. Upload each table once per GPU instead and hand every engine on that device a shared pointer to it. The cache is keyed by (device, source-vector address) because workers are pinned round-robin across GPUs, so on a multi-GPU node each device keeps its own copy - a kernel may only read memory resident on the device it runs on - and the table is freed on the device that allocated it. Entries are held weakly, so a table goes away with the last engine using it. Measured on an 18 Mpx detector, 32 worker threads, 16 GB card: the stills path went from exhausting the card (OOM in de-novo indexing) to 8.6 GB peak, and a normal rotation run from 14.6 GB to 7.4 GB - it had been running within 1.6 GB of the limit, so any larger detector or second GPU consumer would have tipped it over. Per-worker footprint drops 403 -> 173 MB. Merge statistics are unchanged on a six-crystal regression subset, including two-pass runs where the second pass rebuilds the mapping on refined geometry, and wall time is unchanged (13.5-13.8 s vs 13.8-14.1 s). Also take the launch configuration from the current device rather than device 0 in AzIntEngineGPU and ImagePreprocessorGPU: with round-robin pinning, device 0's SM count and shared-memory size can belong to a different card than the one the kernels use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 18:41:27 +02:00
leonarski_fandClaude Opus 5	1a1e05ad14	spot_finding: run the same two passes on the CPU as on the GPU ... ImageSpotFinderGPU::Detect launches its kernel twice, feeding the first pass's strong-pixel bitmap back in so the second recomputes each local background with those pixels excluded and keeps them strong. The CPU finder ran a single pass, so the two returned different spot lists for the same frame and a dataset processed without a GPU did not match one processed with it. It matters for any spot wide enough to reach into its own 31x31 background box: the spot inflates the mean and variance it is then tested against, so its outer pixels fail the SNR test. On the test image added here - a 5x5 core at 300 counts with a one-pixel ring at 25 - a single pass returns the 25-pixel core and 7500 counts where two passes return the full 49 pixels and 8100. pxl_val also becomes int64_t, matching the GPU's pixel_result signature. It was int32_t, so pxl_val * pxl_val overflowed above 46341 counts even though the surrounding sums were already 64-bit. The new parity test compares PixelCount and Count, not just the centroid, which does not move for a symmetric spot whether or not the ring was picked up; it was confirmed to fail against the old single-pass CPU. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 15:37:22 +02:00
leonarski_fandClaude Opus 5	bc5e5d3a3d	image_pusher: do not hold connections_mutex across sends and joins ... The header has said since it was written that blocking queue operations must never run under connections_mutex; three code paths did exactly that. KeepaliveThread held it while sending a keepalive to every connection, which blocks until the peer-liveness or backpressure timeout - so one half-dead writer socket could stall SendImage and every /statistics poll for up to a minute, from an idle-time heartbeat. AcceptorThread and StartDataCollection held it across RemoveDeadConnections, which joins a writer thread that may itself be inside such a send. RemoveDeadConnections is split in two: DetachDeadConnections unlinks them from the pool under the mutex, which is quick, and CloseDeadConnections tears them down afterwards with the mutex released - safe because they are no longer reachable by anyone else. The keepalive loop copies the pool out and sends outside the lock, the pattern EndDataCollection already used. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 15:37:22 +02:00
leonarski_fandClaude Opus 5	84489b63b4	broker: deactivate even when the last run left an error behind ... Deactivate() called measurement.get() inside the try that guards the power-off, so an exception stored by a previously failed run was rethrown before services.Off() ever ran: the detector stayed powered while the state reported Error, and the operator had no way to turn it off. The future is still reaped - it has to be - but its failure is logged and dropped. It was already reported when it happened, and leaving the detector on is the worse outcome. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 15:36:46 +02:00
leonarski_fandClaude Opus 5	797a28a572	rugnux: correct the stale adaptive-spots default comment ... The declaration still said "stills on, rotation off". Adaptive detection has since been turned on for both workflows - adaptive_spots.value_or(true) - which the comment at the assignment already explains. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 15:03:33 +02:00
leonarski_fandClaude Opus 5	b5b7cf2cf9	docs: say what the CPU prediction path actually does ... BraggPrediction.h claimed the buffer "GROWS to whatever a frame actually predicts, so a large cell is never truncated here". Only the two GPU Calc overrides call GrowCapacity; both CPU predictors stop at max_reflections. The cap is applied inside the h/k/l walk and before the resolution test, so what survives is the low-|h| block, not the reflections nearest the Ewald sphere - a cell large enough to overflow 20000 gives different merged reflections with and without a GPU. Documented rather than silently claimed otherwise. Also removed a paragraph describing a once-per-predictor overflow warning that no longer exists, and fixed the rugnux_cli.cpp path in HDF5.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 15:02:57 +02:00
leonarski_fandClaude Opus 5	66e705c0fe	docs: correct the FPGA analysis limits and the square-root coefficient ... The azimuthal bin limit is FPGA_INTEGRATION_BIN_COUNT = 2048, not 1024. There are 16 ROIs, not 64, and the map is a 16-bit per-pixel mask, so a pixel belongs to any subset of them rather than to exactly one. The lossy transform is round(sqrt(N*N*X)) = round(N*sqrt(X)): the HLS squares the sqrtmult register before multiplying. The doc said sqrt(N*X), which is off by sqrt(N), and the register comment claimed the value was "minus one" and "should be square of the coeff" - both wrong, the host writes N and the FPGA squares it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 15:02:11 +02:00
leonarski_fandClaude Opus 5	164f15c903	geom_refinement: stop committing refinements that did not converge ... Four of the seven ceres::Solve calls in image_analysis obtained a Solver::Summary and never looked at it, so a solve that failed numerically had its parameters written back and was reported as success. StillsPartialityRefine and both PostRefine solves already gated on IsSolutionUsable(); this brings the rest to the same contract. IsSolutionUsable() is the right test rather than checking for CONVERGENCE: it accepts a solve that ran out of iterations or wall-clock time but still descended, which is exactly what the real-time callers depend on when they set max_solver_time instead of max_num_iterations. Only FAILURE and USER_FAILURE are rejected. XtalOptimizer checks before the write-back, so a failed refinement now leaves the caller's geom and latt untouched instead of half-updated. GeometryRefiner folds it into result.ok, which previously reported success from spot and frame counts alone. RingOptimizer returns a geometry by value that both callers assign straight back over their input, so it hands back the unchanged reference rather than a diverged beam centre. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 15:02:11 +02:00
leonarski_fandClaude Opus 5	b1740935d8	CLAUDE.md: bring it back in line with the tree ... Verified section by section. The corrections that matter: - image_analysis/pixel_refinement/ is gone (38ea0ec23). The style rule it anchored - no defensive or unrequested code - stays, now attached to the experimental analysis code generally. - rugnux_cli.cpp lives in rugnux/, not tools/ (f737424bd). - There is no .clang-tidy in the tree and never has been; the naming conventions are kept, described as what the code already does. - compression/ has no sqrt codec - the algorithms are BSHUF_LZ4 and the three BSHUF_ZSTD variants. The square-root transform is an FPGA pipeline stage. - jfjoch_hdf5_test is defined in tools/, not tests/. - JFJOCH_VIEWER_ONLY was undocumented, and is forced ON on Windows/macOS. - The per-image-scalar recipe pointed at reader/JFJochHttpReader.cpp, which does not exist (it is viewer/), and missed that EndMessage carries both a per-image vector and a run-mean scalar, so the CBOR END block needs two keys. Added the camelCase-dataset vs snake_case-field trap. - The portability notes described work already done (libjpeg-turbo) and recommended fetching Eigen, which CMakeLists explicitly rules out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:59:13 +02:00
leonarski_fandClaude Opus 5	4ea5e6d01b	viewer: stop rebuilding closed windows on every dataset tick ... The queue-level fix for the live-follow OOM bounded how many datasets are in flight, but not what each tick costs. Three handlers did full-dataset or full-detector work per tick regardless of whether their window was open: - the calibration window copied the whole pixel mask (GetMask returns a reference; it was taken by value), memcpy'd it and ran a full-resolution recolour on the GUI thread; - the image-list window rebuilt one row of eight QStandardItems per image, and then repainted every cell of the model on every frame to move a one-row highlight; - the dataset-info plot was rebuilt twice per tick, because setCurrentIndex fires currentIndexChanged -> comboBoxSelected -> UpdatePlot and the caller then called UpdatePlot again. The first two now defer to showEvent while hidden, following the pattern JFJochViewerReciprocalSpaceWindow::rebuildGL already uses; the highlight repaints only the two rows that change; and the combo is blocked around setCurrentIndex so the plot is built once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:58:35 +02:00
leonarski_fandClaude Opus 5	a6c9f92c24	viewer: cull and cap the saturated-pixel overlay ... DrawSaturation walked the whole saturated set adding two QGraphicsLineItems each, with none of the viewport culling DrawSpots and DrawPredictions do directly above it, and no upper bound - and it is rebuilt on every pan, zoom and frame change. Unlike spots, that set is not bounded by a setting: an over-exposed frame or a missing beamstop saturates a large fraction of the detector, which meant hundreds of thousands of scene items and a multi-second freeze on each mouse drag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:58:35 +02:00
leonarski_fandClaude Opus 5	fb0272023e	scale_merge: apply outlier rejection to the anomalous split on the GPU path ... The GPU merge kernel rejects outliers on the device and keeps a per-full flag there, but only returned the per-group counts. The host array the CPU path fills stayed all zero, and the anomalous I(+)/I(-) accumulator is host-side and unconditional - so with --reject-outliers and a GPU present, the observations the merged IMEAN dropped were still averaged into I(+) and I(-). The same command on a CPU-only host excluded them: the exported anomalous differences depended on whether a GPU was there. R_meas was unaffected, having its own device-side path that reads the flags in place. MergeAccum now hands the per-full flags back so every host-side reduction sees the same rejections. The comment claiming reject_outliers was excluded from the GPU path was never true. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:53:17 +02:00
leonarski_fandClaude Opus 5	f8beee7e87	frontend: surface spot-finding upload failures and send sliders on release ... This was the only panel firing the generated call directly, with the rejection routed to console.log. Because the poll then kept returning the unchanged server value - which still equalled lastDownloadedS - the resync effect never fired, so the dashboard showed a threshold the broker was not using, indefinitely and silently. It now goes through useUpload like its siblings, so a failure raises the snackbar, and onError puts the server's value back in the panel. The eight sliders also applied from onChange, which MUI fires for every intermediate position while dragging: one drag across the ice-ring width sent ~200 PUTs plus ~200 forced /statistics refetches, each reconfiguring spot finding on the running acquisition. Dragging now only moves the panel; the value is sent from onChangeCommitted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:50:39 +02:00
leonarski_fandClaude Opus 5	16145cd55c	compression: bound the hperf decompressor by the buffer it was given ... JFJochDecompressHperfPtr took source_size and never looked at it: every per-block length was read out of the stream and passed straight to LZ4_decompress_safe/ZSTD_decompress as the source length, with src_ptr advanced by it. The only check happened after the whole buffer had already been walked. A truncated frame, or a block header claiming 0x7fffffff, read far past the end of a heap buffer - reachable from the ZeroMQ CBOR path and from any HDF5 chunk the XDS plugin is handed. block_size == 0 satisfied the "% BSHUF_BLOCKED_MULT" test and then divided at nelements / block_size, and source_size < 12 underflowed source_size - 12 to about 2^64. Both are now rejected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:47:01 +02:00