codecov[bot]
commented
4 months ago Codecov Report
Attention: Patch coverage is 83.33333% with 57 lines in your changes missing coverage. Please review.
Project coverage is 36.45%. Comparing base (
52e76d3) to head (b92fae1).
| Files | Patch % | Lines |
|---|---|---|
| include/FastCaloSim/Geometry/Cell.h | 50.00% | 50 Missing :warning: |
| include/FastCaloSim/Geometry/RTree.h | 93.75% | 5 Missing :warning: |
| test/source/AtlasGeoTests.cxx | 96.15% | 2 Missing :warning: |
Additional details and impacted files
```diff @@ Coverage Diff @@ ## main #31 +/- ## ========================================== + Coverage 35.08% 36.45% +1.37% ========================================== Files 218 224 +6 Lines 10575 10893 +318 Branches 1536 1557 +21 ========================================== + Hits 3710 3971 +261 - Misses 6865 6922 +57 ```:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
This pull requests adds a new way to handle the cell lookup. The old lookup code employs very home-cooked and convoluted solutions in
The new approach uses 3D R-Trees as underlying data structure to enable a fast and efficient neighbouring cell lookup. The tree is build in RTree.h and uses boost::geometry::index::rtree under the hood. In principle, we only need the 2D lookup, but no significant difference in timing is observed with respect to the 3D case. Compared to the old code, the R-tree approach seems to be up to a factor 100 slower:
Weather this has any significant impact on the simulation speed remains to be seen. In the current implementation, the generated number of hits scales with energy depending on the intrinsic resolution of the calorimeter technology:
For low to mid-energies, the number of cell lookups should therefore remain small and not contribute to the simulation speed significantly. For high energies, the number of cell lookups is large so it could become the bottleneck in simulation time. However, the relative gain in speed with respect to geant4 at high energies is also much larger, so a slight increase in simulation time for very high energies can be potentially tolerated:
In any case, we will need to allow for custom lookup solutions. For example, we will not be able to do the FCAL lookup in ATLAS with the assumption of regular cell cuboids in x,y,z as the real cell boundaries do not follow exact cuboids in x,y,z:
We will therefore need to allow for custom hooks that experiments can implement if they need more detailed lookups. If the cell lookup will ever be a bottleneck, this can be addressed by custom solutions from experiments.
For now, the old lookup is still used in the simulation, but will be removed in the future. The new cell lookup is tested in AtlasGeoTests, by generating 100 random points within the PreSamplerB (eta, phi, r), EME1 (eta, phi, z) and FCAL0 (x, y, z) layers, and checking if the generated point is within the found cell.