pybind11_add_module links pybind11::lto, which adds full -flto to the
extension and ignores the target's INTERPROCEDURAL_OPTIMIZATION property.
That link ran single-threaded for 19-22 s and was the longest step of the
build. LTO cannot reach aare_core (a plain static library) and the hot
loops are header templates compiled at -O3 inside the extension anyway.
Pass NO_EXTRAS so the standard CMake property decides (off by default),
and re-add the extras pybind11 would have applied: pybind11_strip for
optimized builds and pybind11::windows_extras (/bigobj, /MP) on MSVC.
set_property(TARGET _aare PROPERTY INTERPROCEDURAL_OPTIMIZATION ON) or
-DCMAKE_INTERPROCEDURAL_OPTIMIZATION=ON brings LTO back as ThinLTO.
Measured on a clean Release build with tests: 42 s -> 28 s, extension
link 19 s -> 0.1 s, exports identical, all tests pass. The stripped
binary grows from 4.7 MB to 5.6 MB. Pedestal, eta and the aare_core code
paths (fits, calibration, decoding) run at the same speed; the
ClusterFinder micro-benchmark is consistently 1-2 % slower without LTO.
python/src/module.cpp was one translation unit instantiating every cluster
size and value type; it compiled for 38 s and was the critical path of the
whole build. The bindings now live in bind_io.cpp (files, geometry, pixel
maps, calibration, test probes), bind_hist.cpp, bind_pedestal.cpp,
bind_fit.cpp, bind_eta.cpp and one bind_cluster_NxN.cpp per cluster size,
with the entry points declared in bindings.hpp and called from module.cpp
in the same order as before. The DEFINE_CLUSTER_BINDINGS macros moved to
bind_cluster_common.hpp.
Headers that several units include needed small fixes: allocate_image_data
and define_corner_enum are inline, the file-reader entry point is not, and
np_helper.hpp, bind_Defs.hpp, bind_FastPedestal.hpp,
bind_PedestalTrackingPixelHistogram.hpp and bind_ClusterFileSink.hpp now
include what they use, in particular pybind11/stl/filesystem.h for the
std::filesystem::path arguments they bind (previously supplied by include
order in module.cpp).
The module's exported names, types and docstrings are identical (verified
by dumping all 1948 of them), both test suites pass, and a clean Release
build with tests drops from 68 s to 42 s; the longest unit now takes 13 s
and the remaining 19 s is pybind11's LTO link.
Minuit2's ~60 small files took 45 s of summed compile time in a Release
build (32 s in Debug); batched into unity sources that drops to 9 s (5 s)
with all [fit] tests passing. libzmq gets the same treatment on its
"objects" OBJECT library for when AARE_ZMQ is on: 37 s -> 7 s of CPU,
same warnings, identical exported symbols apart from one implicitly
defined destructor that is now inlined at its single use.
All git-fetched dependencies are shallow clones like Catch2 already was.
A fresh configure with network access went from 13.4 s to 10.5 s here and
the checkouts shrink from 19 MB to 3.7 MB (fmt) and 15 MB to 3.7 MB
(pybind11).
- renamed pedestal.hpp to bind_Pedestal.hpp for python bindings
- variance is now always calculated in double and made private
- cost correctness in a few places
- removed push_fast and explicit updates of mean from Pedestal. If
performance is needed use FastPedestal
- Pedestal no longer caches std
Optimized pedestal tracking with FastPedestal and performance improvements to the cluster finder.
- ~2x faster hitting 14k FPS on benchmark dataset on 16 core threadripper pro
**Changes**
- No bounds check on interior of frame. (only within half of a cluster)
(~15% improvement)
- Cache threshold (std*n_rms) Biggest improvement comes from not
computing std for each access
- Cache pedestal subtracted frame
- Switched to FastPedestal which assumes we reached steady state
**Things tried but rejected:**
- Always store cluster values, commit on val==max (~15% drop in frame
rate)
**Options**
- using 16 bit clusters and 16 bit pedestal. (slows down the single
threaded case but allow us to reach 12k with multi threading)
**Other notes on performance**
- Passing in memory frames from python and doing nothing: 0.8us/frame
- Passing + pedestal subtraction: 28us/frame
- Passing + pedestal + cluster finding (no store): 652us/frame
- Passing + pedestal + cluster finding: 886 us/frame
- Reading files from nfs shares tops out at ~1GB/s for single threaded
reads.
- MultiThreadedFileReader uses our File wrapper to read a generic file
in parallel
- Placed in aare::experimental to show that it's not production ready
If AARE_FETCH_MINUIT is set to OFF we first look for a standalone
Minuit2 and if that is not found we try to find Minuit2 as a part of
ROOT.
In both cases we make an alias to allow for simpler use of the target
later.
It still doesn't solve the issue that we install Minuit to when we fetch
it but that can be addressed in a separate PR.
closes #316
To improve codebase quality and reduce human error, this PR introduces
the pre-commit framework. This ensures that all code adheres to project
standards before it is even committed, maintaining a consistent style
and catching common mistakes early.
Key Changes:
- Code Formatting: Automated C++ formatting using clang-format (based on
the project's .clang-format file).
- Syntax Validation: Basic checks for file integrity and syntax.
- Spell Check: Automated scanning for typos in source code and comments.
- CMake Formatting: Standardization of CMakeLists.txt and .cmake
configuration files.
- GitHub Workflow: Added a CI action that validates every Pull Request
against the pre-commit configuration to ensure compliance.
The configuration includes a [ci] block to handle automated fixes within
the PR. Currently, this is disabled. If we want the CI to automatically
commit formatting fixes back to the PR branch, this can be toggled to
true in .pre-commit-config.yaml.
```yaml
ci:
autofix_commit_msg: [pre-commit] auto fixes from pre-commit hooks
autofix_prs: false
autoupdate_schedule: monthly
```
The last large commit with the fit functions, for example, was not
formatted according to the clang-format rules. This PR would allow to
avoid similar mistakes in the future.
Python fomat with `ruff` for tests and sanitiser for `.ipynb` notebooks
can be added as well.
## Unified Minuit2 fitting framework with FitModel API
### Models (`Models.hpp`)
Consolidate all model structs (Gaussian, RisingScurve, FallingScurve)
into a
single header. Each model provides: `eval`, `eval_and_grad`, `is_valid`,
`estimate_par`, `compute_steps`, and `param_info` metadata. No Minuit2
dependency.
### Chi2 functors (`Chi2.hpp`)
Generic `Chi2Model1DGrad` (analytic gradient) templated on the model
struct.
Replaces the separate Chi2Gaussian, Chi2GaussianGradient,
Chi2Scurves, and Chi2ScurvesGradient headers.
### FitModel (`FitModel.hpp`)
Configuration object wrapping `MnUserParameters`, strategy, tolerance,
and
user-override tracking. User constraints (fixed parameters, start
values, limits)
always take precedence over automatic data-driven estimates.
### Fit functions (`Fit.hpp`)
- `fit_pixel<Model, FCN>(model, x, y, y_err)` -> single-pixel,
self-contained
- `fit_pixel<Model, FCN>(model, upar_local, x, y, y_err)` -> pre-cloned
upar for hot loops
- `fit_3d<Model, FCN>(model, x, y, y_err, ..., n_threads)` ->
row-parallel over pixel grid
### Python bindings
- `Pol1`, `Pol2`, `Gaussian`, `RisingScurve`, `FallingScurve` model
classes with
`FixParameter`, `SetParLimits`, `SetParameter`, and properties for
`max_calls`, `tolerance`, `compute_errors`
- Single `fit(model, x, y, y_err, n_threads)` dispatch replacing the old
`fit_gaus_minuit`, `fit_gaus_minuit_grad`, `fit_scurve_minuit_grad`,
etc.
### Benchmarks
- Updated `fit_benchmark.cpp` (Google Benchmark) to use the new FitModel
API
- Jupyter notebooks for 1D and 3D S-curve fitting (lmfit vs Minuit2
analytic)
- ~1.8x speedup over lmfit, near-linear thread scaling up to physical
core count
---------
Co-authored-by: Erik Fröjdh <erik.frojdh@psi.ch>
- added rosenblatttransform
- added 3x3 eta methods
- interpolation can be used with various eta functions
- added documentation for interpolation, eta calculation
- exposed full eta struct in python
- disable ClusterFinder for 2x2 clusters
- factory function for ClusterVector
---------
Co-authored-by: Dhanya Thattil <dhanya.thattil@psi.ch>
Co-authored-by: Erik Fröjdh <erik.frojdh@psi.ch>
Erik Fröjdh
maliakal_d
lunin_landmazzol_a
