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.

Sign up for GitHub

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

Jump to bottom

refactor!: Track EDM brush over #3192

Merged
merged 18 commits into from
Jul 10, 2024
Merged

refactor!: Track EDM brush over #3192

merged 18 commits into from
Jul 10, 2024

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented May 16, 2024 •
edited
Loading

  • use calibrated instead of measurement
  • expose typedefs
  • correct index params

@andiwand andiwand added this to the v36.0.0 milestone May 16, 2024
@andiwand andiwand mentioned this pull request May 16, 2024
Merged
@github-actions github-actions bot added the Component - Plugins Affects one or more Plugins label May 16, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented May 16, 2024 •
edited
Loading

📊: Physics performance monitoring for 3dba00e

Full contents

physmon summary

@codecov Codecov
Copy link

codecov bot commented Jun 3, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 75.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 47.65%. Comparing base (cf9d872) to head (c8ce9be).
Report is 17 commits behind head on main.

❗ Current head c8ce9be differs from pull request most recent head 304e582

Please upload reports for the commit 304e582 to get more accurate results.

Files Patch % Lines
...e/include/Acts/EventData/VectorMultiTrajectory.hpp 40.00% 3 Missing and 3 partials ⚠️
Core/src/TrackFitting/GainMatrixUpdater.cpp 0.00% 0 Missing and 1 partial ⚠️
Core/src/TrackFitting/GsfUtils.cpp 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3192      +/-   ##
==========================================
+ Coverage   47.24%   47.65%   +0.41%     
==========================================
  Files         508      507       -1     
  Lines       30041    29207     -834     
  Branches    14586    14012     -574     
==========================================
- Hits        14192    13918     -274     
+ Misses       5375     5265     -110     
+ Partials    10474    10024     -450

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andiwand andiwand requested a review from paulgessinger June 24, 2024 12:45
@andiwand andiwand marked this pull request as ready for review June 24, 2024 12:45
@acts-project-service
Copy link
Collaborator

acts-project-service commented Jun 24, 2024 •
edited
Loading

🔴 Athena integration test results [fa78ed3]

Build job with this PR failed!

Please investigate the build job for the pipeline!

@acts-project-service acts-project-service added the Breaks Athena build This PR breaks the Athena build label Jun 24, 2024
@andiwand andiwand mentioned this pull request Jun 26, 2024
Merged
@andiwand andiwand removed the Breaks Athena build This PR breaks the Athena build label Jul 1, 2024
@paulgessinger
Copy link
Member

I guess it does in fact break Athena compilation, doesn't it?

@andiwand
Copy link
Contributor Author

andiwand commented Jul 1, 2024

I guess it does in fact break Athena compilation, doesn't it?

Yes I think at least the measurement -> calibrated method name change will break Athena. I can patch this tomorrow before we eventually merge this

@acts-project-service acts-project-service added the Breaks Athena build This PR breaks the Athena build label Jul 6, 2024
@andiwand andiwand removed the Breaks Athena build This PR breaks the Athena build label Jul 9, 2024
@acts-project-service acts-project-service added the Breaks Athena build This PR breaks the Athena build label Jul 9, 2024
@andiwand
Copy link
Contributor Author

andiwand commented Jul 9, 2024

This seems to break with rootcling in the Athena build e.g. https://gitlab.cern.ch/acts/acts-athena-ci/-/jobs/40986739

Most likely because root is picking the wrong ACTS headers which are not compatible with the changes here.

Maybe we just merge this PR as the last one for v36 or directly before a new nightly is triggered

cc @paulgessinger

@andiwand andiwand added 🛑 blocked This item is blocked by another item and removed Breaks Athena build This PR breaks the Athena build labels Jul 9, 2024
paulgessinger
paulgessinger reviewed Jul 10, 2024
View reviewed changes
Copy link
Member

@paulgessinger paulgessinger left a comment

Choose a reason for hiding this comment

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

Just one more minor thing.

Core/include/Acts/TrackFinding/MeasurementSelector.ipp Show resolved Hide resolved
@andiwand andiwand removed the 🛑 blocked This item is blocked by another item label Jul 10, 2024
@kodiakhq kodiakhq bot merged commit fa78ed3 into acts-project:main Jul 10, 2024
49 checks passed
@andiwand andiwand deleted the refactor-track-edm-brush branch July 10, 2024 21:23
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
4 Accepted issues

Measures
0 Security Hotspots
68.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@acts-project-service acts-project-service added the Breaks Athena build This PR breaks the Athena build label Jul 10, 2024
paulgessinger pushed a commit that referenced this pull request Aug 8, 2024
@andiwand
feat: Variable size measurement for Examples (#3107)
3a8a0e3 
This is an attempt to implement a variable size measurement as an
alternative to the fixed size measurement. Currently our Examples
framework is using a variant over fixed size measurements to cope with
measurements of different dimensions. This obfuscates the code a bit
since accessing the actual measurement requires a visitor pattern to
acquire the dimensional typed measurement.

Here I propose to switch this out for a variable size measurement which
allocates the same amount of memory but stores the dimension as a direct
member instead of pushing this detail to `std::variant`.

blocked by
- #3192
- #3331
kodiakhq bot pushed a commit that referenced this pull request Sep 4, 2024
@andiwand
refactor!: Template algorithms on track container frontend `TrackCont…
1e855a2 
…ainer` (#3193)

Instead of pulling all the track container template parameters into the algorithms we can template on the `TrackContainer` and get the details from there.

blocked by
- #3192
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulgessinger paulgessinger paulgessinger approved these changes

Assignees
No one assigned
Labels
Breaks Athena build This PR breaks the Athena build Component - Core Affects the Core module Component - Plugins Affects one or more Plugins Event Data Model Track Finding Track Fitting
Projects
None yet
Milestone
v36.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@andiwand @acts-project-service @paulgessinger