From b1740935d857c795983043b1bfc75ba09ddfed0e Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Fri, 31 Jul 2026 14:59:13 +0200 Subject: [PATCH] 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) --- CLAUDE.md | 162 +++++++++++++++++++++++++++++++++--------------------- 1 file changed, 100 insertions(+), 62 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1d238217..3ea9c517 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -15,8 +15,11 @@ usage message is the source of truth, not the docs. ## Build Out-of-source CMake build, C++20, heavy use of `FetchContent` (spdlog, zstd, HDF5, -slsDetectorPackage, Catch2, cpp-httplib, libzmq, Ceres, fast-feedback-indexer are downloaded -and statically linked — the **first configure needs network access** and is slow). +slsDetectorPackage, Catch2, cpp-httplib, libzmq, libtiff, FFTW, Ceres, fast-feedback-indexer are +downloaded and statically linked — the **first configure needs network access** and is slow). +Two dependencies are deliberately **not** vendored and must come from the system: **ZLIB** and +**Eigen3 ≥ 3.4**, both resolved with `find_package` (see the Eigen note in `CMakeLists.txt` for why). +libjpeg-turbo is built via `ExternalProject` in `preview/`; libcurl is fetched only for viewer builds. ``` mkdir build && cd build @@ -25,16 +28,24 @@ make -j$(nproc) jfjoch_broker # the main service; build other targets by na ``` Key CMake options: -- `JFJOCH_USE_CUDA` (default ON) — GPU path. Needs CUDA ≥ 12.8. Provides the `ffbidx` and `fft` - GPU indexers; without it only the CPU `fftw` indexer is available (requires FFTW at configure - time, auto-detected). CUDA absence is not a build error. +- `JFJOCH_USE_CUDA` (default ON) — GPU path. Needs CUDA ≥ 12.8 (older is warned about and + ignored). Provides the `ffbidx` and `fft` GPU indexers; without it only the CPU `fftw` indexer is + available. FFTW is fetched and `JFJOCH_USE_FFTW` defined unconditionally, so `fftw` is always + there. CUDA absence is not a build error — nvcc is looked for on `PATH`, `CUDA_PATH` and + `/usr/local/cuda`. - `JFJOCH_WRITER_ONLY` (default OFF) — builds only the HDF5 writer; skips broker, FPGA, receiver, analysis, tests, frontend. -- `JFJOCH_VIEWER_BUILD` (default OFF) — builds the Qt6 `jfjoch_viewer` desktop app. +- `JFJOCH_VIEWER_BUILD` (default OFF) — builds the Qt6 `jfjoch_viewer` desktop app in addition to + the server stack. +- `JFJOCH_VIEWER_ONLY` (default OFF on Linux, **forced ON on Windows/macOS**) — builds only + `jfjoch_viewer`, `rugnux` and the libraries they link; skips receiver, FPGA, detector control, + tests and the frontend. - `SLS9` (default OFF) — build against slsDetectorPackage 9.2.0 instead of 8.0.2. +- `JFJOCH_INSTALL_DRIVER_SOURCE` (default OFF) — install the PCIe driver source for DKMS/RPM. -The frontend is a separate custom target: `make frontend` (runs `npm ci && npm run build` in -`frontend/`). It is not built by default unless installing. +The frontend is a separate custom target: `make frontend` (in `frontend/`: `npm ci`, +`npm run build`, plus the third-party-licenses, Redoc and Sphinx-docs bundling steps). It is never +built automatically — `make install` only copies whatever already sits in `frontend/dist/`. ## Test @@ -49,11 +60,14 @@ cd tests ./jfjoch_test -r junit -o report.xml ``` -`make jfjoch_hdf5_test` builds the HDF5 write-speed benchmark, also used by CI to produce files -that are validated against XDS (Durin/Neggia) and CrystFEL. +`make jfjoch_hdf5_test` builds the HDF5 write-speed benchmark (it lives in `tools/`, so the binary +is `build/tools/jfjoch_hdf5_test`); CI also uses it to produce files that are validated against XDS +(Durin/Neggia), DIALS and CrystFEL. `jfjoch_hdf5_enospc_test` + the `enospc_shim` module test +out-of-space handling. -Lint config is `.clang-tidy` (broad `*` check set with many exceptions; namespaces lower_case, -classes CamelCase, global constants UPPER_CASE). +There is no lint config in the repo (no `.clang-tidy`/`.clang-format`, no CI lint step) — follow the +conventions the code already uses: namespaces lower_case, classes CamelCase, global constants +UPPER_CASE. ## Code style @@ -62,10 +76,10 @@ implementation that a reader can verify at a glance. Extra abstraction, speculat clever-but-dense constructs are treated as actively harmful, not as polish. When torn between a tidy abstraction and a flat, obvious version, pick the obvious one. -This matters most in `image_analysis/pixel_refinement/` (e.g. `PixelRefine.cpp`), which is an -experimental prototype where readability is how the physics gets verified — keep it especially -plain. **Do not add defensive/unrequested code** (extra validation, rejection heuristics, "just in -case" branches) without asking first; if a guard isn't clearly needed, leave it out. +This matters most in the experimental analysis code under `image_analysis/`, where readability is +how the physics gets verified — keep those parts especially plain. **Do not add +defensive/unrequested code** (extra validation, rejection heuristics, "just in case" branches) +without asking first; if a guard isn't clearly needed, leave it out. Match the surrounding code's idiom, naming, and comment density rather than importing a different style. @@ -115,8 +129,9 @@ cd tests/test_data && python jfjoch_broker_test.py # feeds a test image, sta **Data flow (online):** detector → FPGA acquisition (`fpga/`, `acquisition_device/`) → `receiver/` builds full images from per-module FPGA output → `image_pusher/` streams CBOR-encoded -images over ZeroMQ → `jfjoch_writer` (`writer/`) consumes the stream and writes NXmx HDF5. The -broker also emits a low-rate preview stream and a metadata stream (`preview/`). +images over ZeroMQ (or TCP) → the consuming side (`image_puller/`) feeds `jfjoch_writer` +(`writer/`), which writes NXmx HDF5. The broker also emits a low-rate preview stream and a +metadata stream (`preview/`). **Writer file split:** one acquisition produces one `_master.h5` plus many `_data_NNNNNN.h5` files. Dataset-wide metadata (geometry, detector config, ROI/azimuthal definitions — anything @@ -129,7 +144,9 @@ The HDF5 master/data layout is one of three `FileWriterFormat`s (`common/JFJochM all NXmx: **`NXmxLegacy`** (master + `_data_NNNNNN.h5` joined by external links), **`NXmxVDS`** (master + data joined by HDF5 virtual datasets — the default), and **`NXmxIntegrated`** (a single self-contained file, no separate data files). Per-image plugins -must work for all three; with `NXmxIntegrated` "master" and "data" are the same file. +must work for all three; with `NXmxIntegrated` "master" and "data" are the same file. (The enum +also has non-NXmx `DataOnly` and `NoFile`; values 4/5 are retired CBF/TIFF, kept only in the +OpenAPI enum for back compatibility.) **Two acquisition workflows:** the FPGA-accelerated path (JUNGFRAU at PSI; FPGA does masking, summation, spot finding, ROI/azimuthal integration, compression) and the DECTRIS SIMPLON path @@ -146,30 +163,38 @@ generated API model and internal types. `JFJochImageAnalysis`): - `jfjoch_broker` — online, real-time (FPGA + GPU). - `jfjoch_viewer` — interactive Qt desktop (`viewer/`), results not persisted. -- `rugnux` (`tools/rugnux_cli.cpp`, built on the `Rugnux` library in `rugnux/`) — offline batch over - a stored HDF5; writes `_process.h5` and `.mtz`/`.cif`/`.hkl`. Merging is on by default (`--no-merge` - to disable); `--azint-only` runs only azimuthal integration and `--scale` re-scales/merges the - already-integrated reflections in a `_process.h5`. (rugnux = the data-processing half of the system; - see `docs/NAMING.md`.) +- `rugnux` (`rugnux/rugnux_cli.cpp`, built on the `Rugnux` library in the same directory) — offline + batch over a stored HDF5, invoked as `rugnux {} ` (it has **no `--help`**; run + it with no arguments to print the usage, which is the authority on its flags). Rotation vs stills + is auto-detected from the goniometer axis. Merging is on by default (`--no-merge` to disable); + merging writes `.mtz`/`.cif`/`.hkl` and skips the bulky `_process.h5` unless `--write-process-h5`, + while `--no-merge` writes only `_process.h5`. `--azint-only` runs only azimuthal integration, and + `--scale` re-scales/merges the already-integrated reflections in a `_process.h5`. (rugnux = the + data-processing half of the system; see `docs/NAMING.md`.) **`image_analysis/` pipeline** (subdirs): `spot_finding`, `indexing` (`ffbidx`/`fft` GPU, -`fftw` CPU), `lattice_search`, `geom_refinement`, `pixel_refinement`, `bragg_prediction`, -`bragg_integration`, `rotation_indexer`, `azint`, `roi`, `scale_merge`. Least-squares refinement -uses **Ceres** (fetched, built with miniglog, no MKL, CXX_THREADS). `ffbidx` needs a known cell -(`-C`) and suits sparse serial stills; `fft`/`fftw` index de novo and suit strong rotation data. +`fftw` CPU), `lattice_search`, `geom_refinement`, `bragg_prediction`, `bragg_integration`, +`image_preprocessing`, `azint`, `roi`, `scale_merge`, plus `rotation_indexer/` and +`dark_mask_analysis/` compiled straight into `JFJochImageAnalysis`. (`beam_stop/` is an unbuilt +prototype — it is in no `CMakeLists.txt`.) Least-squares refinement uses **Ceres** (fetched in +`image_analysis/CMakeLists.txt`, built with miniglog, no MKL, no Ceres-CUDA, CXX_THREADS). +The indexer is chosen with `-X` (`FFBIDX|FFT|FFTW|Auto|None`, default `Auto`, which resolves to a +GPU indexer when one is present and `fftw` otherwise): `ffbidx` wants a known cell (`-C`) and suits +sparse serial stills; `fft`/`fftw` index de novo and suit strong rotation data. **FPGA** (`fpga/`): `hls/` is the Vitis HLS source (image-analysis kernels), `hls_simulation/` -runs that same HLS on CPU for hardware-free testing, `host_library/` is the host-side driver, -`pcie_driver/` is the kernel module. The HLS algorithms are documented in +runs that same HLS on CPU for hardware-free testing, `hdl/` is the Verilog RTL, `host_library/` is +the host-side driver, `pcie_driver/` is the kernel module. The HLS algorithms are documented in `docs/FPGA_DATA_ANALYSIS.md`. **Detector control** (`detector_control/`): wrappers for SLS (JUNGFRAU) and DECTRIS SIMPLON (EIGER). **`jungfrau/`**: JUNGFRAU ADU→energy gain/pedestal calibration. **Other libs:** `common/` (geometry, diffraction experiment, image buffer, CUDA wrappers — the -shared core, linked nearly everywhere), `compression/` (zstd + bitshuffle + sqrt lossy), -`frame_serialize/` (CBOR stream codec), `gemmi_gph/` (vendored GEMMI for MTZ/XDS_ASCII I/O), -`xds-plugin/` (XDS HDF5 read plugin). +shared core, linked nearly everywhere), `compression/` (vendored bitshuffle + LZ4 + zstd; the +algorithms are `BSHUF_LZ4`, `BSHUF_ZSTD`, `BSHUF_ZSTD_RLE`, `BSHUF_ZSTD_RLE_HUFF`), +`frame_serialize/` (CBOR stream codec), `reader/` (HDF5 dataset read-back), `gemmi_gph/` (vendored +GEMMI for MTZ/XDS_ASCII I/O), `xds-plugin/` (XDS HDF5 read plugin). ## Portability (jfjoch_viewer) @@ -190,15 +215,16 @@ portable; don't constrain it for portability's sake. configuration). This non-CUDA path must keep working, but it is the macOS fallback — *not* the intended Windows configuration. -A self-contained Windows build still needs a few dependencies that the Linux build picks up from -the system and that are **not** auto-provided via `FetchContent` (besides Qt, supplied -externally): **ZLIB** (pulled by hdf5/libtiff/gemmi/libzmq — a bundled zlib-ng in `ZLIB_COMPAT` -mode is the intended fix, presented as the `ZLIB::ZLIB` target so the scattered -`find_package(ZLIB)` calls resolve), **libjpeg-turbo** (used by `preview/`; upstream discourages -`add_subdirectory`, so bring it in via `ExternalProject`), and **Eigen** (header-only; needed by -Ceres, by the analysis libs directly, and by `ffbidx` under CUDA — fetch it and point Ceres' -internal `find_package(Eigen3)` at it). Wire each guarded so the Linux build, which finds these on -the system, stays unchanged. +`JFJOCH_VIEWER_ONLY` is forced ON on Windows and macOS, so a plain configure there already builds +just the portable subset. libtiff, libjpeg-turbo (via `ExternalProject` in `preview/`) and libcurl +(viewer builds only) are now brought in by the build itself, so they need nothing from the host. +Two dependencies are still supplied externally on every platform, deliberately and by decision, not +as an open TODO: **ZLIB** and **Eigen** (header-only; needed by Ceres, by the analysis libs +directly, and by `ffbidx` under CUDA). Both are resolved with `find_package` — the system package on +Linux (`zlib-devel`, `eigen3-devel`), a build prefix pointed at by `CMAKE_PREFIX_PATH`/`Eigen3_DIR` +on Windows. Vendoring Eigen through `FetchContent` with `OVERRIDE_FIND_PACKAGE` is specifically +ruled out: it segfaults the CMake bundled with Visual Studio (see the long note in `CMakeLists.txt`). +Qt is supplied externally as before. ## OpenAPI is the single source of truth @@ -209,13 +235,16 @@ the system, stays unchanged. - TypeScript frontend client → `frontend/src/client/` (hey-api `openapi-ts`, `npm run openapi`). When you change `jfjoch_api.yaml`, regenerate the relevant client(s); for a version bump write the new version into `VERSION` and run -`update_version.sh` (which reads it and rewrites `frontend/src/version.ts`, `docs/conf.py`, the -python client and the Redoc html). +`update_version.sh` (which reads it and rewrites the `version:` in the YAML itself, +`frontend/src/version.ts`, `frontend/package.json`, `docs/conf.py`, the python client, the Redoc +html — and also the FPGA HDL and PCIe-driver version strings). It downloads +`openapi-generator-cli.jar` and runs `npm install`, so it needs network access. ## Frontend -React 19 + TypeScript + MUI + Vite (`frontend/`). Data layer is generated from the OpenAPI spec -(`@hey-api/openapi-ts` → fetch client + TanStack Query hooks + zod schemas). Scripts: +React 19 + TypeScript + MUI 6 + Vite 7 (`frontend/`); charts are Plotly (`react-plotly.js`). Data +layer is generated from the OpenAPI spec (`@hey-api/openapi-ts` → fetch client + TanStack Query +hooks + zod schemas; config in `frontend/openapi-ts.config.ts`). Scripts: `npm start` (dev server), `npm run build` (tsc + vite), `npm run openapi` (regen client), `npm run redocly4broker` (regen `broker/redoc-static.html`). @@ -227,29 +256,38 @@ A per-image scalar (e.g. `ice_ring_score`, `bkg_estimate`, `mosaicity`) flows an 1. **Compute** where the azint profile is finalized: `image_analysis/MXAnalysisWithoutFPGA.cpp` (CPU), `receiver/JFJochReceiverFPGA.cpp` (FPGA), and the offline azint worker in `rugnux/Rugnux.cpp`. -2. **Message** (`common/JFJochMessages.h`): `std::optional` in `DataMessage`, `std::vector` - in `EndMessage`. -3. **CBOR**: encode in `frame_serialize/CBORStream2Serializer.cpp` (DataMessage block *and* END block), - decode in `CBORStream2Deserializer.cpp` (both). Optional fields are back-compatible — no version bump. +2. **Message** (`common/JFJochMessages.h`): `std::optional ` in `DataMessage`; in + `EndMessage` **two** members — `std::vector v_` (the per-image array) and an + `std::optional ` run-mean scalar. Mind the `v_` prefix. +3. **CBOR**: encode in `frame_serialize/CBORStream2Serializer.cpp` — one key in the DataMessage block + (`SerializeImageInternal`) *and* two in the END block (`SerializeSequenceEnd`: `` and + `v_`); decode the same three in `CBORStream2Deserializer.cpp`. Optional fields are + back-compatible — no version bump. 4. **HDF5** write: `writer/HDF5DataFilePluginMX.{h,cpp}` — an `AutoIncrVector` with reserve / per-image write / `SaveVector("/entry/MX/")` (per-image arrays live in the data-file plugin); - plus the NXmx master write in `writer/HDF5NXmx.cpp` (`SaveVectorIfMissing(..., end.)`). HDF5 - **read-back** (so a stored file re-opens, e.g. in the viewer) is in `reader/HDF5MetadataSource.cpp`, - NOT `JFJochHDF5Reader.cpp`: mirror the three `bkgEstimate` sites — master `ReadOptVector`, data-file - `ReadVector` into the dataset, and the per-image message population. + plus the NXmx master writes in `writer/HDF5NXmx.cpp` (`SaveVectorIfMissing(..., end.v_)` and + a `SaveScalar(... "Mean", end.)`). **HDF5 dataset names are camelCase even though the message + fields are snake_case** — `bkg_estimate` is stored as `/entry/MX/bkgEstimate` (+ `bkgEstimateMean`). + HDF5 **read-back** (so a stored file re-opens, e.g. in the viewer) is in + `reader/HDF5MetadataSource.cpp`, NOT `JFJochHDF5Reader.cpp`: mirror the three `bkgEstimate` sites — + master `ReadOptVector`, data-file `ReadVector` into the dataset, and the per-image message population. 5. **Scan result**: `common/ScanResult.h` (`ScanResultElem`) + `common/ScanResultGenerator.cpp` - (copy in `Add`, resize+fill in `FillEndMessage`). -6. **Receiver plot**: `common/Plot.h` (`PlotType`) + `common/JFJochReceiverPlots.{h,cpp}` (`StatusVector` - + Clear / AddElement / GetPlots / GetPlotRaw cases). + (copy in `Add`, resize+fill in `FillEndMessage`). The name is **not** preserved here — the + `bkg_estimate` member is called just `bkg`, and so is the API property in step 7. +6. **Receiver plot**: `common/Plot.h` (`PlotType`) + `common/JFJochReceiverPlots.{h,cpp}` — a + `StatusVector` member, cleared in `Setup`, fed in `Add`, plus the `GetPlots` / `GetPlotRaw` cases + and (if the run-mean scalar of step 2 is wanted) a `GetBkgEstimate`-style accessor. 7. **API**: add to the `plot_type` enum and the `scan_result` images schema in `broker/jfjoch_api.yaml`, regenerate the C++ model (`java -jar openapi-generator-cli.jar generate -i broker/jfjoch_api.yaml -o broker/gen -g cpp-pistache-server`) and the frontend client (`cd frontend && npm run openapi`), then wire `broker/OpenAPIConvert.cpp` (`ConvertPlotType` string→enum and the `Convert(ScanResult)` setter). -8. **Reader/viewer**: `reader/JFJochReaderDataset.h` + `reader/JFJochHttpReader.cpp` (`GetPlot_i`) and +8. **Reader/viewer**: `reader/JFJochReaderDataset.h` + `viewer/JFJochHttpReader.cpp` (`GetPlot_i`) and `viewer/JFJochViewerDatasetInfo.cpp` (combo item + `ExtractMetric`). 9. **Frontend**: `frontend/src/components/DataProcessingPlots.tsx` (`MenuItem`) + `DataProcessingPlot.tsx` - (y-axis label). -10. **Docs**: `docs/CBOR.md`, `docs/HDF5.md`, `docs/CPU_DATA_ANALYSIS.md`. + in the same directory (y-axis label in `AxisTypeY`). +10. **Docs**: `docs/CBOR.md` and `docs/HDF5.md` name the fields literally; `docs/CPU_DATA_ANALYSIS.md` + describes the quantity in prose. -Gotcha: an existing `build/` dir needs a `cmake .` reconfigure to pick up a newly-added `broker/gen` -source file (the source list is a configure-time glob). +Gotcha: an existing `build/` dir needs a `cmake .` reconfigure to pick up a newly-added +`broker/gen/model` source file — `broker/CMakeLists.txt` collects it with `AUX_SOURCE_DIRECTORY`, +a configure-time directory scan. (`gen/api` is include-path-only and needs no reconfigure.)