Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: Introduce regular surface intermediate class #2340

Merged
merged 33 commits into from
Nov 11, 2023

Conversation

paulgessinger
Copy link
Member

@paulgessinger paulgessinger commented Aug 2, 2023

This adds a new class

class RegularSurface : public Surface {};

that reimplements a number of functions while discarding the direction argument.

@paulgessinger paulgessinger added this to the next milestone Aug 2, 2023
@github-actions github-actions bot added Component - Core Affects the Core module Component - Fatras Affects the Fatras module Component - Examples Affects the Examples module Component - Plugins Affects one or more Plugins Event Data Model Vertexing Track Fitting labels Aug 2, 2023
@paulgessinger
Copy link
Member Author

I extracted the changes to normal and the introduction of coerceToSurface to #2344, so we can evaluate whether this is a good idea separately.

@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

Merging #2340 (d2f6c21) into main (c3ceb12) will increase coverage by 0.09%.
The diff coverage is 69.64%.

@@            Coverage Diff             @@
##             main    #2340      +/-   ##
==========================================
+ Coverage   49.61%   49.71%   +0.09%     
==========================================
  Files         473      474       +1     
  Lines       26844    26859      +15     
  Branches    12362    12362              
==========================================
+ Hits        13319    13352      +33     
+ Misses       4753     4706      -47     
- Partials     8772     8801      +29     
Files Coverage Δ
Core/include/Acts/Detector/Portal.hpp 100.00% <ø> (ø)
...e/include/Acts/Digitization/DigitizationModule.hpp 66.66% <ø> (ø)
Core/include/Acts/Digitization/Segmentation.hpp 100.00% <ø> (ø)
Core/include/Acts/Geometry/VolumeBounds.hpp 40.00% <ø> (ø)
Core/include/Acts/Surfaces/ConeSurface.hpp 100.00% <ø> (ø)
Core/include/Acts/Surfaces/CylinderSurface.hpp 100.00% <ø> (ø)
Core/include/Acts/Surfaces/DiscSurface.hpp 100.00% <ø> (ø)
Core/include/Acts/Surfaces/LineSurface.hpp 100.00% <ø> (ø)
Core/include/Acts/Surfaces/PerigeeSurface.hpp 100.00% <ø> (ø)
Core/include/Acts/Surfaces/PlaneSurface.hpp 100.00% <ø> (ø)
... and 15 more

... and 4 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@paulgessinger paulgessinger modified the milestones: v30.0.0, v31.0.0 Sep 26, 2023
@acts-project-service acts-project-service added the Breaks Athena build This PR breaks the Athena build label Oct 25, 2023
@paulgessinger
Copy link
Member Author

This is rebased again. Should we just merge it @andiwand?

andiwand
andiwand previously approved these changes Nov 8, 2023
andiwand
andiwand previously approved these changes Nov 8, 2023
@paulgessinger
Copy link
Member Author

Ok I might have to fix some compile issues and also the Athena build seems to be failing on this in an unexpected way.

@paulgessinger
Copy link
Member Author

Ready again now I think @andiwand.

@kodiakhq kodiakhq bot merged commit 85a0fea into acts-project:main Nov 11, 2023
53 checks passed
@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label Nov 14, 2023
@paulgessinger paulgessinger removed Breaks Athena build This PR breaks the Athena build Fails Athena tests This PR causes a failure in the Athena tests labels Nov 16, 2023
kodiakhq bot pushed a commit that referenced this pull request Nov 21, 2023
This helps catching missing `using` methods to make derived classes fully comply with the base `Surface` class. C++ inheritance is strange.

Blocked by:
- #2340
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking change This change breaks backwards compatibility Component - Core Affects the Core module Component - Documentation Affects the documentation Component - Fatras Affects the Fatras module Component - Plugins Affects one or more Plugins Event Data Model Track Fitting Vertexing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants