refactor: Brush over new detector geometry part 1

[andiwand]

• consistent pass by value
• reformat documentation

pull changes out of #2214

[github-actions]

📊 Physics performance monitoring for 335038a

Summary
Full report
Seeding: seeded, truth estimated, orthogonal
CKF: seeded, truth smeared, truth estimated, orthogonal
IVF: seeded, truth smeared, truth estimated, orthogonal
AMVF: seeded, truth smeared, truth estimated, orthogonal
Ambiguity resolution: seeded, orthogonal
Truth tracking
Truth tracking (GSF)

Vertexing

Vertexing vs. mu

IVF seeded

IVF truth_smeared

IVF truth_estimated

IVF orthogonal

AMVF seeded

AMVF truth_smeared

AMVF truth_estimated

AMVF orthogonal

Seeding

Seeding seeded

Seeding truth_estimated

Seeding orthogonal

CKF

CKF seeded

CKF truth_smeared

CKF truth_estimated

CKF orthogonal

Ambiguity resolution

seeded

Truth tracking (Kalman Filter)

Truth tracking

Truth tracking (GSF)

Truth tracking

[asalzburger]

Looks mostly cosmetics, fine with me.

[codecov]

Codecov Report

Merging #2309 (335038a) into main (7251c28) will increase coverage by 0.01%.
The diff coverage is 86.36%.

@@            Coverage Diff             @@
##             main    #2309      +/-   ##
==========================================
+ Coverage   49.49%   49.51%   +0.01%     
==========================================
  Files         450      450              
  Lines       25461    25465       +4     
  Branches    11720    11718       -2     
==========================================
+ Hits        12603    12608       +5     
  Misses       4565     4565              
+ Partials     8293     8292       -1     

Impacted Files	Coverage Δ
Core/include/Acts/Detector/Portal.hpp	100.00% <ø> (ø)
.../include/Acts/Navigation/DetectorVolumeFinders.hpp	71.42% <ø> (ø)
...include/Acts/Navigation/DetectorVolumeUpdators.hpp	71.42% <ø> (ø)
Core/src/Detector/Detector.cpp	60.00% <75.00%> (+2.02%)	⬆️
Core/src/Detector/LayerStructureBuilder.cpp	37.38% <75.00%> (+1.80%)	⬆️
Core/src/Detector/DetectorVolume.cpp	49.36% <85.71%> (ø)
Core/include/Acts/Detector/DetectorVolume.hpp	74.28% <100.00%> (ø)
Core/include/Acts/Navigation/NextNavigator.hpp	47.65% <100.00%> (ø)
Core/src/Detector/Portal.cpp	71.66% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[paulgessinger]

Isn't it usually safer to use constant refs, because that decreases the likelihood that the caller accidentally copies? Especially with vector arguments I've seen that happen for sure. Might not be super relevant for the geometry construction.

[andiwand]

Isn't it usually safer to use constant refs, because that decreases the likelihood that the caller accidentally copies? Especially with vector arguments I've seen that happen for sure. Might not be super relevant for the geometry construction.

Not sure - you lose the ability to move the vector to where it belongs and will always end up with a copy. clang tidy will check the intermediate steps so should not be forgotten. The caller might forget it but then you have a single copy as before

[paulgessinger]

Isn't it usually safer to use constant refs, because that decreases the likelihood that the caller accidentally copies? Especially with vector arguments I've seen that happen for sure. Might not be super relevant for the geometry construction.

Not sure - you lose the ability to move the vector to where it belongs and will always end up with a copy. clang tidy will check the intermediate steps so should not be forgotten. The caller might forget it but then you have a single copy as before

Fair enough. Maybe we should take some extra care for vectors that are large or contain very complex objects, but that doesn't seem to be the case here anyway.

[asalzburger]

Looks like we have an agreement here, I see it as Andreas does, the PR makes this consistent and hence allows copy or move, whatever is better.

[acts-project-service]

🔴 Athena integration test results

🔴 Some tests have failed!

Please investigate the pipeline!

status	job
🟢	run_unit_tests
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsWorkflow.sh
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateTracks.sh
🔴	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateActsCoreSpacePoints.sh
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateActsTrkSpacePoints.sh
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateSeeds.sh
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateOrthogonalSeeds.sh
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsValidateClusters.sh
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsPersistifyEDM.sh
🟢	run_ci_tests: ../athena/AtlasTest/CITest/test/ActsKfRefitting.sh
🟢	run_ci_tests: python3 ../athena/Tracking/Acts/ActsGeometry/test/ActsExtrapolationAlgTest.py
🟢	run_ci_tests: python3 ../athena/Tracking/Acts/ActsGeometry/test/ActsITkTest.py
🟢	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

[CarloVarni]

I believe the failure is coming from Athena and is un-related to this PR

[asalzburger]

None of this code is running in Athena already.