feat: Bind some Gen1 geometry building functionality to python

[benjaminhuth]

Gen1 Geometry building ist better than its reputation, if you bind it to python. Was used to build ITk from GeoModel.

Necessary changes to the GeoModel Plugin will come in a subsequent PR, the script that actually builds the ITk won't live in the ACTS repository.

[benjaminhuth]

cc @asalzburger @noemina

[paulgessinger]

Sorry to push back on this a little bit, but I don't think we should substantially increase the amount of Core API surface that we expose to python, and then implement logic that can be implemented in C++ in python instead.

The python bindings are supposed to steer the examples, and help with configuration, not contain sophisticated logic and algorithms.

Aside from this, I see a number of unrelated changes, that should be broken out into one or more separate PRs, in my opinion.

[paulgessinger]

Can we hold this until Thursday so we can discuss how to proceed with this?

[benjaminhuth]

Unfortunately I won't be there on thursday, and also not in the first week of August. But maybe we can resolve this online...

Regarding the question if it at all should be in python and cannot be in C++: I think that adding bindings to a few building structures does not substentially increase the API surface, and is well justified in that case: I don't think its good to have details like regex parsing of ITK database strings, number of layers, radii of volumes etc. in C++ rather then in a python script. I think the chosen approach is actually quite in the spirit of having configuration logic in python, and generic functionality in C++.
Last but not least I'm not super eager to rewrite the code in C++ now where it is already done in python^^

In todays standup meeting we also clarified, that the build script itself will not become part of the ACTS repository, but rather the ITk repository where also the database is located. Also from this perspective I think a python script is favourable over a C++-compiled version of the geometry building.

[benjaminhuth]

For the record: build script has been removed, unrelated/propagation related changes have been reverted

[github-actions]

📊: Physics performance monitoring for dec0a63

Full contents

physmon summary

• ✅ Particles fatras
• ✅ Particles geant4
• ✅ Particles ttbar
• ✅ Vertices ttbar
• ✅ Truth tracking (KF)
• ✅ Truth tracking (GSF)
• ✅ Truth tracking (GX2F)
• ✅ CKF | trackfinding | single muon | truth smeared seeding
• ✅ Track Summary CKF | trackfinding | single muon | truth smeared seeding
• ✅ Seeding trackfinding | single muon | truth estimated seeding
• ✅ CKF | trackfinding | single muon | truth estimated seeding
• ✅ Track Summary CKF | trackfinding | single muon | truth estimated seeding
• ✅ Seeding trackfinding | single muon | default seeding
• ✅ CKF | trackfinding | single muon | default seeding
• ✅ Track Summary CKF | trackfinding | single muon | default seeding
• ✅ Seeding trackfinding | single muon | orthogonal seeding
• ✅ CKF | trackfinding | single muon | orthogonal seeding
• ✅ Track Summary CKF | trackfinding | single muon | orthogonal seeding
• ✅ Seeding trackfinding | 4 muon x 50 vertices | default seeding
• ✅ CKF | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ Track Summary CKF | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ Ambisolver | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ IVF notime | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ AMVF gauss notime | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ AMVF grid time | trackfinding | 4 muon x 50 vertices | default seeding
• ✅ Seeding trackfinding | ttbar with 200 pileup | default seeding
• ✅ CKF | trackfinding | ttbar with 200 pileup | default seeding
• ✅ Track Summary CKF | trackfinding | ttbar with 200 pileup | default seeding
• ✅ Ambisolver | trackfinding | ttbar with 200 pileup | default seeding
• ✅ AMVF gauss notime | trackfinding | ttbar with 200 pileup | default seeding
• ✅ AMVF grid time | trackfinding | ttbar with 200 pileup | default seeding

[paulgessinger]

@benjaminhuth Sure.

Mainly I'm concerned with creating volume bounds and tracking volumes directly in Python, and doing any of the sizing calculations in Python.

Especially the latter clearly deviates from configuration over calculation. If that's outside the repository and hence specific to the ITk test, I'm ok with that.

If you ask me, I would suggest to find a way to put the sizing logic into C++ with some form of configuration at the boundary and bind that to Python.

[benjaminhuth]

I think the main difference here in our approach is that I only aime for "making it work until NextGen Geometry is ready", so the approach was never to port the full flexibility of Gen1 geometry building to python. I don't think we open the box of pandora with that.

I think it would be far from straight forward to do a generic C++ sizing logic for Gen1 geometry (I think that is done in the GenericDetector to some degree, but turned out to not be portable enough for this use case). Trying this would end up in an almost as complex system as the blueprint method or so... And I would not invest time in such a thing for an already deprecated geometry model.

In my opinion it is be okay to have incomplete and inconsistent bindings for this for the time being, just make things work in this geometry transistion phase that will be anyways a bit unpleasent...

[paulgessinger]

I see your point. But to some degree the thing is that yes, the sizing logic it possibly non-trivial, I wrote the code to do that in Athena for the Gen1 ACTS geometry.

I think it would be far from straight forward to do a generic C++ sizing logic for Gen1 geometry

Correct, the interfaces were meant to be used in experiment specific code to write geometry construction. The dd4hep and TGeo plugins achieve this through configuration, which is necessary because we ship them as plugins.

GeoModel is a bit different because it's only used by ATLAS, but I don't think we should ship a plugin that's "GeoModel, but only for ITk kind of".

Now, with the Python building code gone from the repository, I don't have fundamental objections against the rest of this PR.

I would still encourage evaluating the code that we do put in, so that it makes sense, and is not just whatever needed to be added to get something done.

[benjaminhuth]

@paulgessinger @andiwand I changed the PR as follows:

• Changes to the GeoModel Plugin are postponed to a subsequent PR
• The PythonLogger is not used anymore

Is this good to go in from your perspective?

[benjaminhuth]

I think I addressed your comments, can you have a look again @paulgessinger ?

[sonarcloud]

Quality Gate failed

Failed conditions
9.5% Coverage on New Code (required ≥ 25%)
33.33% Line Coverage on New Code (required ≥ 50%)

See analysis details on SonarCloud

[paulgessinger]

Merging this manually.

[acts-project-service]

✅ Athena integration test results [ecad6fd]

✅ All tests successful

status	job	report
🟢	report_pull_request
🟢	run_art_test: test_data18_13TeV_1000evt
🟢	run_art_test: test_ttbarPU40_reco
🟢	test_ActsDumpGeometryIdentifiers
🟢	test_ActsCheckObjectCountsCachedWorkflow
🟢	test_ActsCheckObjectCountsWorkflow
🟢	test_ActsEFTrackFit
🟢	test_ActsPersistifySeeds
🟢	test_ActsBenchmarkWithSpot
🟢	test_ActsAnalogueClustering
🟢	test_ActsWorkflowHeavyIons
🟢	test_ActsWorkflowFastTracking
🟢	test_ActsWorkflowCached
🟢	test_ActsWorkflow
🟢	test_ActsValidateAmbiguityResolution
🟢	test_ActsValidateResolvedTracks
🟢	test_ActsValidateTracks
🟢	test_ActsValidateActsCoreSpacePoints
🟢	test_ActsValidateActsSpacePoints
🟢	test_ActsValidateSeeds
🟢	test_ActsValidateOrthogonalSeeds
🟢	test_ActsValidateClusters
🟢	test_ActsPersistifyEDM
🟢	test_ActsGSFRefitting
🟢	test_ActsKfRefitting
🟢	test_ActsExtrapolationAlgTest
🟢	test_ActsITkTest
🟢	run_workflow_tests_run4_mc
🟢	run_workflow_tests_run2_mc
🟢	run_workflow_tests_run2_data
🟢	run_workflow_tests_run3_mc
🟢	run_workflow_tests_run3_data
🟢	run_unit_tests