fix: Forward missing config args to the seeders in python - #6052
Open
benjaminhuth wants to merge 2 commits into
Open
benjaminhuth wants to merge 2 commits into
benjaminhuth wants to merge 2 commits into
Conversation
addGridTripletSeeding and addOrthogonalTripletSeeding hardcoded several GridTripletSeedingAlgorithm::Config / OrthogonalTripletSeedingAlgorithm::Config fields to None -- silently dropping them instead of forwarding the values already carried on SeedFinderConfigArg / SeedFilterConfigArg / SeedingAlgorithmConfigArg. Others were never mentioned at all, even though both namedtuples and the C++ Config already had a field for them. A config written as if it were being honoured therefore silently ran with different, defaulted parameters -- only visible as two different configs producing identical output. Now forwarded for both triplet seeding functions: deltaZMin/deltaZMax, helixCutTolerance, toleranceParam, deltaInvHelixDiameter. GridTriplet additionally forwards numPhiNeighbors, zBinNeighborsTop/Bottom, and falls back to seedFinderConfigArg.zBinEdges when spacePointGridConfigArg.zBinEdges is unset (matching zBinsCustomLooping's existing seedFinderConfigArg source). New namedtuple fields: deltaZMin, helixCutTolerance, toleranceParam on SeedFinderConfigArg; deltaInvHelixDiameter on SeedFilterConfigArg. deltaZMin defaults to minus deltaZMax rather than the C++ default of -inf when only deltaZMax is given: deltaZ is a signed doublet cut in DoubletSeedFinder, and the legacy scalar deltaZMax alone historically meant the symmetric "maximum absolute deltaZ" -- forwarding it without a matching deltaZMin would silently turn a symmetric cut into a one-sided one. Also fixes an adjacent latent bug found while touching this code: addOrthogonalTripletSeeding was passing phiBinDeflectionCoverage, maxPhiBins, zBinEdges and zBinsCustomLooping as kwargs, none of which exist on OrthogonalTripletSeedingAlgorithm::Config (it is KD-tree based, not grid based). This only worked because those namedtuple fields default to None and get dropped; setting any of them would raise a TypeError at construction. Removed; the newly-forwarded numPhiNeighbors/zBinNeighborsTop/Bottom are correspondingly only added to addGridTripletSeeding, for the same reason. Deliberately out of scope: seedFilterConfigArg.seedConfirmation is still not read as a fallback by either function (matches addStandardSeeding's two-source behaviour), so the in-tree ITk config in itk.py continues to run GridTriplet with confirmation off. Several config-arg fields have no home in either C++ Config at all (binSizeR, maxPtScattering, deltaPhiMax, allowSeparateRMax, seedFilterConfigArg.deltaRMin, spacePointGridConfigArg.rMax/deltaRMax/ impactMax) and are left silently dropped, as before. Adds test_seeding_config_routing.py, pinning every forwarded field end-to-end through both functions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018BdDCi2FLjxjZ7xWFFBEU1
|
Contributor
Public API surface diffNo change to the public API surface. ✅ |
andiwand
approved these changes
Sep 8, 2026
Contributor
benjaminhuth
marked this pull request as ready for review
September 9, 2026 09:21
benjaminhuth
enabled auto-merge
September 9, 2026 09:21
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.