1291 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
leonarski_fandClaude Opus 5	d615b900dd	ci: correct the tools the viewer archive is said to carry, and two dead conditions ... rugnux_scale and azint have not been separate binaries since they became rugnux --scale and rugnux --azint-only; the viewer component carries rugnux, jfjoch_extract_hkl and jfjoch_recompress. Two job conditions tested github.ref_type against 'workflow_dispatch'. ref_type is only ever 'branch' or 'tag', so that test never matched and never did anything: build-rpm's whole condition was that test, and half of the unit-test one was. Both jobs ran on a dispatch, as the release flow needs them to. Drop the dead tests rather than repair them - the behaviour they read as intending is not the behaviour that is wanted - and write down why unit-tests is skipped on a tag, which is the part that is deliberate: the dispatch run is the one that tests, and the tag it creates only rebuilds and uploads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:53:04 +02:00
leonarski_fandClaude Opus 5	0dd837e8e4	rugnux: name the third correction surface in the usage message ... --no-scaling-corrections said it disabled the decay and absorption surfaces. It disables the modulation surface too - GetCorrectionSurfaces() gates all three - and the flag has worked that way since the detector-plane modulation was added; only its description did not follow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:52:41 +02:00
leonarski_fandClaude Opus 5	6ae4cfc9e5	writer: take --verbose without an argument, and say what -d takes ... --verbose was declared required_argument while -v carries no colon in the getopt string, so the long form consumed whatever followed it. "jfjoch_writer --verbose tcp://host:5400" swallowed the address as the flag's argument and then failed for want of a data source - the short form was fine, which is presumably why it went unnoticed. The usage line for the root directory asked for <int>. It is a path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:52:41 +02:00
leonarski_fandClaude Opus 5	8712160adb	scripts: point the generated Python client at gitea, and make the scripts runnable ... The openapi-generator invocation still passed --git-host=git.psi.ch and a user id of jungfraujoch, from before the move to gitea.psi.ch/mx/jungfraujoch. Those properties are not cosmetic: they become the source URL in the generated README and pyproject, so the published client documentation - docs/python_client/README.md, which is copied out of the generated tree - told readers to pip install from a host that no longer answers. Regenerating with the corrected flags changes those two lines and nothing else, verified against the committed tree. update_version.sh, make_doc.sh and gen_python_client.sh were all mode 644, so the "run update_version.sh" the documentation asks for fails on the shebang. CMake and the CI both work around it by invoking them through bash. make_doc.sh builds a throw-away venv in the working tree and deletes it on the last line, which set -e skips whenever pip or sphinx fails - so a failed docs build left tmp_venv/ behind. Delete it from a trap instead, and ignore it along with the default output directory and the sdist directory gen_python_client.sh creates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:52:29 +02:00
leonarski_fandClaude Opus 5	22d75e2f01	build: take the project version from the VERSION file, and read it once ... PROJECT() carried a hardcoded 1.0.0 next to a JFJOCH_VERSION read from the VERSION file, so PROJECT_VERSION was free to drift from the version everything else uses. It cannot simply be handed the same string - project(VERSION) accepts numeric major.minor.patch only, and rejects a pre-release suffix such as -rc.161 - so cut the numeric part out of the same file instead of writing it down a second time. common/ then read ../VERSION again into PACKAGE_VERSION, purely to interpolate it into GitInfo.cpp. That is the same file read twice with two variable names, one of them a common enough name to be set by something else in the parent scope. Use JFJOCH_VERSION, which is already in scope there. The CUDA architecture note claimed the list "embeds no PTX". A bare entry in CMAKE_CUDA_ARCHITECTURES emits SASS and PTX both, so the newest entry has been the forward-compatibility path all along: on a GPU newer than anything listed, the driver JIT-compiles that PTX at first launch. Adding sm_121 still buys native code on Spark instead of a JIT, which is what the comment should have said. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:52:09 +02:00
leonarski_fandClaude Opus 5	0003acf2e4	packaging: name the Debian broker package jfjoch, and let its DKMS scripts run ... CPACK_DEBIAN_MAIN_COMPONENT does not exist. Only CPackRPM.cmake has a MAIN_COMPONENT variable, which is why the RPM came out as "jfjoch" while the .deb of the same component came out as "jfjoch-jfjoch" - the DEB generator names every component package "<CPACK_PACKAGE_NAME>-<component>" unless CPACK_DEBIAN_<COMPONENT>_PACKAGE_NAME overrides it, and the line we set was read by nobody. It also named a component ("broker") that does not exist, so it could not have matched anything either way. Set the name the generator actually reads, and declare Replaces/Conflicts on the old one: the new package owns the same files, so dpkg would otherwise refuse to unpack it over an installation that already has jfjoch-jfjoch. The DKMS component's postinst and prerm are the driver's postinstall.sh and preuninstall.sh, configure_file'd into place. They were mode 644, and CPACK_DEBIAN_PACKAGE_CONTROL_STRICT_PERMISSION is off, so they were packaged as they are - and dpkg cannot execute a maintainer script it cannot execute. The RPM path is unaffected: it reads the same files as text into the spec. The viewer's Freedesktop menu entry and its icon are installed on Linux only now. On Windows the Start Menu shortcut comes from CPACK_PACKAGE_EXECUTABLES and on macOS from the .app bundle, so on those two the installer was carrying a share/applications entry and a share/pixmaps icon that nothing reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 20:51:55 +02:00
leonarski_fandClaude Opus 5	ceb92fc4cc	docs: bring the rc.161 change list up to date ... Covers the GPU decode work (fused un-transpose, the memory it frees, corrupt-chunk detection, large bitshuffle blocks, host fallback), the two reproducibility fixes (integer ring statistics, striped geometry-refinement sampling), the connection-teardown and written-extent fixes, and the thread-count, max_hkl and compression-time repairs. Folded into the existing entries where they belong rather than added as new ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 16:58:03 +02:00
leonarski_fandClaude Opus 5	d79b20e268	indexing: key the shared device tables on their content, not only on an address ... The cache returned a device copy for a (device, host address) pair and cast it to whatever the caller asked for, with nothing checking that the bytes behind that address were still the same bytes. A host buffer can be mutated in place - PixelMask::LoadMask does exactly that - or freed and reallocated at the same address, and either hands the caller a device copy of something else. Nothing would report it: the tables are read-only geometry, so the engine would simply mask the wrong pixels for the rest of the run while the azimuthal mapping, the written pixel_mask dataset and the viewer overlay used the new one. Today that is unreachable, but only because of two guards in unrelated files that neither state nor assert the requirement. The byte length and an FNV-1a checksum of the bytes being uploaded are now part of the key. Both are computed once per engine construction, over a buffer that is about to be copied to the device anyway, so the cost does not show. Expired entries are pruned on insert, since distinct content now means distinct entries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 16:33:24 +02:00
leonarski_fandClaude Opus 5	6b713cf3da	rugnux: size the written datasets by the ordinals reached, not by the successes ... Each image is written at its own ordinal, so a frame that fails to load or analyse leaves a HOLE - the frames after it keep their positions rather than shifting up. The end message nevertheless reported the number of frames that SUCCEEDED as the image count, and the writer sizes /entry/data/data and every per-image array from that. So one failed frame in the middle of a run made the declared extent one short, and the image it dropped was the LAST one written, not the one that failed. Two failures dropped two, and so on: the file quietly ends before the data does, with the per-image metadata still carrying rows for images the VDS no longer maps. It also under-counted the data files when the run was split. Track the highest ordinal actually written and use that. A hole then reads as the fill value, which is what a frame that was never written should look like, and the counts of collected and written images stay counts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 16:33:24 +02:00
leonarski_fandClaude Opus 5	5727cb68a4	rotation_indexer: write down why the supercell bar is unreachable, and what failed to fix it ... `frac > RATIO * best_frac` cannot be satisfied once best_frac passes 1/RATIO - above 0.667 for a ratio of 1.5, which is ordinary for good rotation data. Above that the two guards do not raise the bar, they close the branch: no axis multiple and no lower-symmetry setting can displace the incumbent however much better it fits, so a genuine superstructure is kept as its sub-cell and its satellite rows go unindexed, silently. The obvious repair - restate the bar on the fraction left UNINDEXED, which is well defined over the whole range - was implemented and measured. It regressed the 37-crystal battery from 34/37 to 32/37 correct space groups: a C2 lattice fell to P1, and a P2 case went to C222 keeping 2923 of 22440 reflections with CC1/2 in the last shell at -35%. The indexed fraction is too noisy to carry a looser test. So the unreachable-but-safe form stays, and the limitation is recorded at the comparison rather than left to be rediscovered. Fixing it properly needs the selection to be decided on something better than the indexed fraction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 15:32:55 +02:00
leonarski_fandClaude Opus 5	abb94ca450	spot_finding: accumulate the adaptive ring statistics in integers ... The per-ring sums were floats reduced by atomics, so the ring sigma - and with it the detection threshold - depended on the order the blocks happened to arrive in. Detection compares an INTEGER pixel value against that threshold, so a threshold that drifts across an integer flips every pixel of that value in the ring at once, which is how a last-bit difference turned into a different spot list. A preprocessed pixel is an exact int32 and the masked and saturated sentinels are skipped, so v and v*v are exact in 64 bits, and integer addition is associative: the sums no longer care about arrival order. Both engines now accumulate the same way, so they agree exactly rather than approximately, and the GPU spot list is bit-identical across runs. The corrected sums that feed the reported azimuthal profile stay float - a pixel value times a float correction has no exact integer form - but they do not enter the detection decision. Cost: the ring reduction needs 28 bytes per bin instead of 20 in the plain pass, which drops it from eight co-resident blocks per SM to seven and costs about 11% of that kernel (0.582 -> 0.650 ms/frame on a 4.5 Mpx frame). End to end it does not show: alternating runs on three rotation crystals came out the same or slightly faster, and the battery is unchanged in every number. The CPU engine got 30% faster (32.2 -> 22.6 ms/frame), integers being cheaper than doubles. Tests: exact CPU/GPU agreement on the spot list, and 50 repeats of bit-identical output where there were four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 15:19:11 +02:00
leonarski_fandClaude Opus 5	212fbf9bab	rugnux: make the geometry-refinement sample deterministic, and spread it over the run ... The stills first pass drew frames from a shared cursor and stopped when a shared counter reached its target, which got two things wrong at once. The cursor walked the equally spaced sample in ascending order, so stopping early read only its leading PREFIX - the beam centre, distance and cell were fitted to the beginning of the run, not across it, and the comment claiming otherwise was wrong. And where the stop landed depended on how the workers happened to interleave, so the set of frames varied run to run: on the same data at -N 32 and -N 8 the pass examined 483 and 457 frames and refined the detector distance to 168.0481 and 168.0530 mm. The sample is now cut into a fixed number of interleaved stripes, each stopping once it has contributed its share. Every stripe spans the whole run, so an early stop no longer biases the fit, and a stripe is processed identically whichever worker claims it - so what gets examined depends only on the data, not on timing and not on -N. The same three runs now give 451 frames examined and 168.0452 mm, identically. The bundle selection was order-dependent too: frames are collected in worker-completion order and sorted by spot count with a non-stable sort, so equally strong frames swapped places between runs. They carry their image ordinal now and it breaks the tie. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 15:19:11 +02:00
leonarski_fandClaude Opus 5	5f8d37cdab	image_pusher: serialise tearing a connection down ... Two futures per connection are joined from more than one path: the acceptor reaping a dead connection, and the control plane starting or ending a run. Calling get() on one future from two threads at once is undefined and invalidates it, and the guard against it was a non-atomic test-then-set of an atomic flag - `if (!c.active) return; c.active = false;` - so both callers could pass it. Holding connections_mutex across the joins is what the previous commit removed on purpose, and rightly: the joins block on a writer that may be inside a send. So the lock is per connection and covers only the teardown. Nothing it joins takes it, so it cannot deadlock. The comment claiming a detached connection is unreachable by anyone else was wrong: a control-plane call that copied the pool before the erase still holds a shared_ptr to it. That is precisely how the two teardowns meet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:19:27 +02:00
leonarski_fandClaude Opus 5	2d3c39c9dd	image_preprocessing: write whole elements out of the un-transpose ... The raw-bytes path assembled each element a byte at a time, which on a full frame cost about 4x against writing the 8 contiguous elements a thread owns through an element-typed pointer. They are 8*ES-byte aligned, so the compiler merges them. 72.4 MB frame: 1.524 -> 0.406 ms for upload plus both kernels. The test now also times the LZ4 pass on its own, so the bounds and validity checks in the hot loop can be costed rather than guessed at. They are free: 0.231 ms against 0.2297 ms measured for the kernel before any of them existed - the restored offset == 1 and power-of-two fast paths pay for them. compute-sanitizer memcheck reports no error over 400 single-bit-corrupted payloads and nine malformed containers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:16:58 +02:00
leonarski_fandClaude Opus 5	b6c4a59d69	docs: record the stored-format break, and say how the adaptive finders actually accumulate ... The per-image image-scale B factor was dropped from the CBOR stream and from the written HDF5, which is a change for anything reading those files, but the changelog listed it only under the OpenAPI breaking changes. The GPU adaptive finder test claimed both finders sum the rings in double. The CPU one does; the GPU one stages a block's contribution in float before reducing across blocks in double, deliberately, to keep the hot loop's shared footprint down. Say so, and say what follows from it - detection compares integer pixel values, so a threshold that crosses an integer flips every pixel of that value in the ring at once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:14:24 +02:00
leonarski_fandClaude Opus 5	4b1c611bdf	image_analysis: query the current device, and upload the resolution mask on the engine stream ... Two leftovers from earlier fixes of the same shape. BraggIntegrationEngineGPU still read device 0's shared-memory size to decide whether its profile grid fits; workers are pinned round-robin across GPUs, so on a heterogeneous node that check can pass on a different card than the one the kernel launches on. SpotExtractorGPU still uploaded its default resolution mask with a pageable copy on the NULL stream, which is not ordered against the engine stream now that streams are created non-blocking. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:14:24 +02:00
leonarski_fandClaude Opus 5	32ecc296b8	broker: bootstrap a concrete max_hkl, as the API and the docs already claim ... How far the predictor walks the lattice became a setting defaulting to "derive it from the refined cell", which is what is wanted offline. Online it is not: per-image prediction cost then depends on whichever crystal is mounted, and the cube (2n+1)^3 grows quickly. The broker was only ever handed a concrete value when a client sent a bragg_integration block, and none of the shipped configs has one, so the derived path was the normal deployment. Four places in the tree - the conversion, the predictor, the OpenAPI description and the changelog - already state that the broker keeps a fixed bootstrap. Make that true. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:14:09 +02:00
leonarski_fandClaude Opus 5	42eb5bbe83	rugnux: clamp the refinement thread count to what the setting accepts ... RefineThreads rejects anything above 64, and the two-pass rotation path - the default - passed nthreads/2 straight from the machine. On a host with 130 or more hardware threads rugnux therefore exited 1 with the bare message "Candidate-cell refinement thread count" before reading a single image, and the only workaround was to discover -N and pass a smaller value. Dual 64-core parts are squarely in range. The limit is now a named constant the setter and the callers share, so the two cannot drift apart again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:14:09 +02:00
leonarski_fandClaude Opus 5	bec7e2e922	image_preprocessing: fuse the bitshuffle inverse with preprocessing, and verify the decode ... The device decoder was byte-exact on every valid input - 994 production-compressed images, 927 hand-built LZ4 blocks covering engineered (offset, matchlen) pairs across the overlap branch boundary, 18000 repeat decodes, sanitizer-clean - and an audit against LZ4_decompress_generic could not construct a valid block it mis-decodes. What it did not do was notice when the input was NOT valid, and that mattered more than it looks: the decode buffers are reused frame to frame, so a block that stopped early left the PREVIOUS image in place, and in the bitshuffled layout the untouched tail is the most significant byte-plane. A corrupt chunk therefore did not look like a missing corner. It looked like thousands of real pixels several powers of two too bright, fed to spot finding with no diagnostic, where the host decoder had raised an error. So the kernel now flags a block that fails to reach its declared length while consuming exactly its payload, and the host turns that into an exception once the caller has synchronised. Reads are clamped against the end of the payload as well as the output, both length chains are bounded exactly as read_variable_length bounds them, the two offset bytes are bounded, and LZ4's parsing restrictions are enforced. On the host side a block size that is not a multiple of 8 elements is rejected (it made the un-transpose read uninitialised shared memory), the block count is bounded by what the chunk could hold before it becomes an allocation (twelve header bytes could demand hundreds of MB of pinned memory, permanently, per worker), trailing bytes are rejected, and the stream is synchronised before any throw that happens after work is queued. An image of fewer than 8 elements is all verbatim tail and now decodes rather than throwing. When the device route fails for any reason the host decoder gets its turn, so it costs speed rather than the acquisition. The lanes cooperate on the copies and a later match can read bytes another lane wrote, which since Volta needs an explicit __syncwarp(); it worked only because ptxas happened to reconverge at the post-dominator. The prototype's offset == 1 and power-of-two fast paths are also restored - the shipped kernel ran a runtime modulo, an emulated 32-bit division per output byte, on the path its own comment calls the common case. The un-transpose is now fused with preprocessing. One thread owns one group of 8 elements across every byte-plane, so once it has transposed its 8 bytes out of each plane it holds 8 complete elements and emits 8 finished int32 pixels with the mask, the error marker, the saturation cap and the statistics applied. The decompressed image is never materialised: 0.623 -> 0.411 ms/frame at 18 Mpx, 0.523 -> 0.340 with 8 concurrent workers. Staging nothing in shared memory also drops the 48 kB ceiling, which had made any file whose bitshuffle blocks exceed it a hard failure; 64 kB blocks now decode. gpu_compressed is sized from the chunk with grow-on-demand instead of from the uncompressed size - it was reserving ~73 MB per worker to hold ~4 MB. Measured on a 1630x1553 uint32 rotation set at -N 32, peak GPU memory falls 3756 -> 3084 MiB; the same model gives ~144 MB per worker on an 18 Mpx frame. Decoding on the device also stopped reporting a decompression time, which blanked the broker's compression plot trace and filled /entry/profiling/compressionTime with NaN. The decoder brackets the decode with CUDA events and reports it again. Tests: a differential fuzz suite against the CPU decoder - incompressible and highly compressible data, engineered offsets, a size sweep hitting every rem%8 value twice, all six element sizes, an 18 Mpx frame, decoder reuse, concurrency, hand-built LZ4 blocks across the overlap boundary, 26 foreign bitshuffle block sizes from 128 B to 64 kB, corrupt payloads and malformed containers, with a coverage report that proves which LZ4 paths were reached rather than assuming it. Plus the fused path held byte for byte against ImagePreprocessorCPU, statistics included, and against the host-upload path on the same frame. Battery: 37 crystals, every merged number identical to the host-decode run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 14:13:55 +02:00
leonarski_fandClaude Opus 5	47277674fa	docs: split the breaking API changes out of the rc.161 change list ... Earlier releases put breaking changes in their own paragraph after the bullets (rc.139, rc.29) rather than as one item among them. Follow that: the OpenAPI changes now sit under their own heading below the list, with the client-side action in the lead line, and are listed one per change instead of run together. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 07:46:57 +02:00
leonarski_fandClaude Opus 5	737cbde3ff	docs: tighten the changelog and the decoding section for release ... The rc.161 changelog had grown entries several hundred words long and listed the same area three or four times over. Collapse them by subject - spot finding, resolution limits, space-group search, scaling, performance, correctness - and hold each to one line, keeping the actionable detail in the breaking API entry. Add the performance work that had not been written up: device-side image decoding and the parallel first-pass candidate-cell refinement. Section 0 of the CPU analysis document was the longest thing in it after two core algorithm sections, and most of that was a profiling narrative and the measurements that motivated the change rather than a description of what runs. Cut it to the two kernels, the host-side block scan and the fallback rule. The attribution stays; it is also in the reference list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 07:44:55 +02:00
leonarski_fandClaude Opus 5	7e47afe47f	rugnux: parallelise candidate-cell refinement, and stop repeating work in the tail ... Three independent changes to the CPU-bound parts of an offline rotation run, none of which alters a result. Candidate-cell refinement now splits across threads. RefineCandidateCells already took a (block, nblocks) partition, but the only call site passed nblocks=1, so the whole first pass of a two-pass rotation run sat on one thread per scheme - two threads, unchanged at every -N, for a third of the run. A block touches only its own scores(j) and cells rows and holds its own scratch, so the split is exact. The budget is a new IndexingSettings::RefineThreads, left at 1 by default and set only where few indexer threads exist: raising it unconditionally would oversubscribe the paths that already run one indexer per image across all workers. The mmCIF writer built a std::ostringstream per formatted number, twelve per reflection. snprintf gives the same digits for 0.535 -> 0.220 s per file. The space-group search built the same orbit mapping twice per candidate point group - once for the merge chi^2 and once for the systematic-error b, an apply_to_hkl and Canonicalize per observation per operator each time. Build it once and hand it to both. 18 Mpx rotation set 24.6 -> 18.7 s, 2.5 Mpx 13.0 -> 10.7 s, and the 37-crystal battery 13m55s -> 10m47s with no failures, the same 34/37 space groups, and statistics unchanged on 30 of 37 (the rest drift within the run-to-run spread the binary already had, which a control build with the split disabled reproduces). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 07:30:33 +02:00
leonarski_fandClaude Opus 5	13aa20a528	bragg_integration: grow the GPU reflection arrays with slack ... EnsureCapacity resized its 13 device arrays to exactly the current image's predicted-reflection count, so every image that set a new record freed and reallocated all of them. cudaMalloc and cudaFree take a device-wide lock in the CUDA driver, so those images stalled every other worker: sampling the worker threads during the per-image loop found 21-24 of 32 parked in cuMemAlloc_v2 or cuMemFree_v2, all called from this one function, and the running maximum makes 32 workers do far more allocator work than one does. Grow by half again instead. All transfers and kernel launches are sized by the per-image reflection count rather than by the capacity, and the member is already documented as holding at least that many, so over-allocating changes no result. On an 18 Mpx rotation set the integration stage drops from 1.37 to 1.25 ms per image at 32 workers; merged statistics, error model and adopted space group are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 01:02:07 +02:00
leonarski_fandClaude Opus 5	6e4c0ce202	image_preprocessing: decode bitshuffle+LZ4 on the GPU ... The pipeline decompressed each image on the host and uploaded the result. On an 18 Mpx rotation dataset that made the host-to-device copy the bottleneck of the whole per-image loop: nsys puts the copies at 78% of the loop against 39% for every kernel combined - 3600 transfers of 72.4 MB - and they ran at only 12.5 GB/s of an available 27-28 because the host-side decompression was itself saturating host memory bandwidth. The GPU was mostly waiting. So the compressed chunk goes across instead, about 4 MB rather than 72 MB, and is decoded on the device. That removes the transfer and the host decompression that was throttling it, in one change. Measured on an idle machine, a run goes from 45.11 s to 24.97 s - 1.81x - with the merged output unchanged. THE APPROACH IS JON WRIGHT'S (ESRF): "Experiences with GPU decompression for bitshuffle + LZ4 data", HDF5 User Group 2021, and github.com/jonwright/ bslz4decoders. The kernels here are ours, but the idea and the demonstration that it is worth doing are his. Cited in docs/ACKNOWLEDGEMENT.md and in the new section 0 of docs/CPU_DATA_ANALYSIS.md. Two kernels mirror the CPU decoder. LZ4 runs one WARP per bitshuffle block: every lane parses the same sequence stream (a broadcast read, no divergence) and the literal and match copies are split across the 32 lanes so the stores coalesce; an overlapping match is treated as a pattern of period offset sourced from bytes that already precede the write position, which keeps it parallel rather than a serial byte loop. One thread per block instead measured 13x slower. The bitshuffle inverse then un-transposes each byte-plane through shared memory and interleaves the planes back into elements. Only BSHUF_LZ4 is decoded on the device. The zstd variants have no device decoder, and neither has an uncompressed or float image; Supports() returns false for those and the caller decompresses on the host exactly as before. The fallback is explicit, so a format we cannot decode on the device is a slower path and never a wrong answer. Tests hold the device decoder against the CPU one byte for byte, on data from the production compressor, for every element size the detectors emit - including the 8-bit DECTRIS modes, which take bitshuf_decode_block's separate elem_size == 1 branch - plus a many-block frame, the formats it must decline, and malformed containers, which must throw rather than run off a buffer. Battery: 37 crystals, no failures, identical to the host-decode run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 00:16:36 +02:00
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