refactor!: Clean up triplet seeding config structs and grid API - #6004
Conversation
A pass over the configuration surface of the triplet seeder and the space point grids. Seeding output is unchanged. The three space point grid configs held the bottom and top bin finder in a `std::optional`, although neither is optional: every constructor calls `.value()` unconditionally, so forgetting one surfaced as an unmessaged `std::bad_optional_access`. They are plain members now, with a default member initializer giving one neighbour on either side per axis, which is what every call site already set. The grids are no longer copyable or movable. `BinnedGroup` keeps pointers to the bin finders, which live in the grid's own copy of the config, and the base class keeps a pointer into the binned group, so a move left both referring to the source object. `spacePointsSortedByRadius`, `cotThetaDiffMax`, `absDeltaEtaWeightFactor` and `absDeltaEtaMinImpact` were missing from the seeding JSON converters and reset to their defaults on a round trip. The converters themselves moved from `Acts::Experimental` to `Acts`, where the types they convert live, so that ADL can find them at all. The rest is consistency. Config validation in the cylindrical and spherical grids throws `std::invalid_argument` like the cartesian one and `BinnedGroup`. The seeding configs use fixed width integers, and the grid dimension comes from the grid type instead of a hard coded `3ul`. `deltaEtaMax` is a bin width, so it is `etaBinSize`. The doc comments for `useStripInfo`, `cotThetaDiffMax` and `computeCoordRange` describe what the code does, and two dead members of `GridTripletSeedingAlgorithm` are gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017H4TWxPuRU6gxSwMfZPfFp
Review guideThe optional. GridBinFinder<GridType::DIM> bottomBinFinder{1, 1, 0};The alternative, The move. Namespace of the JSON converters. They were declared in What the type changes do and do not touch. Behaviour. The only observable differences are the exception type from Checked locally. Full build clean apart from a pre-existing local DD4hep Left for follow-ups. The view parameters of |
andiwand
changed the title
github-actions
Bot
added
Component - Core
Public API surface diff+0 added, 13 breaking.
|
Co-authored-by: Paul Gessinger <hello@paulgessinger.com>
Invalidated by push of e2a3626
|
2 participants
A pass over the configuration surface of the triplet seeder and the space
point grids. Seeding output is unchanged.
The three space point grid configs held the bottom and top bin finder in a
std::optional, although neither is optional: every constructor calls.value()unconditionally, so forgetting one surfaced as an unmessagedstd::bad_optional_access. They are plain members now, with a default memberinitializer giving one neighbour on either side per axis, which is what every
call site already set.
The grids are no longer copyable or movable.
BinnedGroupkeeps pointers tothe bin finders, which live in the grid's own copy of the config, and the base
class keeps a pointer into the binned group, so a move left both referring to
the source object.
spacePointsSortedByRadius,cotThetaDiffMax,absDeltaEtaWeightFactorandabsDeltaEtaMinImpactwere missing from the seeding JSON converters and resetto their defaults on a round trip. The converters themselves moved from
Acts::ExperimentaltoActs, where the types they convert live, so that ADLcan find them at all.
The rest is consistency. Config validation in the cylindrical and spherical
grids throws
std::invalid_argumentlike the cartesian one andBinnedGroup.The seeding configs use fixed width integers, and the grid dimension comes from
the grid type instead of a hard coded
3ul.deltaEtaMaxis a bin width, soit is
etaBinSize. The doc comments foruseStripInfo,cotThetaDiffMaxandcomputeCoordRangedescribe what the code does, and two dead members ofGridTripletSeedingAlgorithmare gone.