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

fix: MultiTrajectory specialize DynamicColumn for bool #1730

Merged
merged 3 commits into from
Dec 12, 2022

Conversation

paulgessinger
Copy link
Member

@paulgessinger paulgessinger commented Dec 9, 2022

As vector is specialized on bool, which prevents getting references to the underlying bool mechanisms, we have to specialize DynamiColumn to break this specialization of vector.

I'm also rolling in another small const-correctness fix to dynamic columns into this PR.

As vector is specialized on bool, which prevents getting references to
the underlying bool mechanisms, we have to specialize `DynamiColumn` to
break this specialization of vector.
@paulgessinger paulgessinger added this to the next milestone Dec 9, 2022
@github-actions
Copy link

github-actions bot commented Dec 9, 2022

📊 Physics performance monitoring for 37e33a7

Full report
Seeding: seeded, truth estimated, orthogonal
CKF: seeded, truth smeared, truth estimated, orthogonal
IVF: 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

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

@paulgessinger
Copy link
Member Author

There are no references to the bools in std::vector<bool>.

Copy link
Member

@benjaminhuth benjaminhuth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@codecov
Copy link

codecov bot commented Dec 12, 2022

Codecov Report

Merging #1730 (37e33a7) into main (ce28f69) will decrease coverage by 0.00%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##             main    #1730      +/-   ##
==========================================
- Coverage   49.27%   49.27%   -0.01%     
==========================================
  Files         398      398              
  Lines       21763    21764       +1     
  Branches     9874     9875       +1     
==========================================
  Hits        10724    10724              
  Misses       4174     4174              
- Partials     6865     6866       +1     
Impacted Files Coverage Δ
Core/src/EventData/VectorMultiTrajectory.cpp 77.20% <ø> (ø)
...e/include/Acts/EventData/VectorMultiTrajectory.hpp 58.20% <33.33%> (-0.31%) ⬇️

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

@kodiakhq kodiakhq bot merged commit 20f0954 into acts-project:main Dec 12, 2022
pbutti pushed a commit to pbutti/acts that referenced this pull request Dec 13, 2022
…1730)

As vector is specialized on bool, which prevents getting references to the underlying bool mechanisms, we have to specialize `DynamiColumn` to break this specialization of vector.

I'm also rolling in another small const-correctness fix to dynamic columns into this PR.
@paulgessinger paulgessinger modified the milestones: next, v22.0.0 Dec 21, 2022
CarloVarni pushed a commit to CarloVarni/acts that referenced this pull request Dec 22, 2022
…1730)

As vector is specialized on bool, which prevents getting references to the underlying bool mechanisms, we have to specialize `DynamiColumn` to break this specialization of vector.

I'm also rolling in another small const-correctness fix to dynamic columns into this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants