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

feat: Memory dense MeasurementContainer for Examples #3528

Merged
merged 23 commits into from
Sep 4, 2024

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented Aug 21, 2024

  • Implement a MeasurementContainer which packs the numbers densely
  • Implement proxy types to read and write from/to the container
  • Handle all downstream changes

The motivation for this is to use the memory efficiently and potentially improve track finding performance in the Examples framework.

blocked by

@andiwand andiwand added this to the next milestone Aug 21, 2024
@github-actions github-actions bot added Component - Core Affects the Core module Component - Examples Affects the Examples module Event Data Model Track Finding labels Aug 21, 2024
This reverts commit 5725e8b.
@andiwand andiwand added the 🛑 blocked This item is blocked by another item label Aug 21, 2024
@andiwand andiwand removed the 🛑 blocked This item is blocked by another item label Sep 1, 2024
@andiwand andiwand changed the title feat: Dense MeasurementContainer for Examples feat: Memory dense MeasurementContainer for Examples Sep 2, 2024
Copy link

github-actions bot commented Sep 2, 2024

📊: Physics performance monitoring for 7ea7bab

Full contents

physmon summary

@andiwand andiwand marked this pull request as ready for review September 2, 2024 13:02
@andiwand
Copy link
Contributor Author

andiwand commented Sep 2, 2024

This seems to be about 1% faster but that is also very close to the measurement precision

image

@andiwand
Copy link
Contributor Author

andiwand commented Sep 2, 2024

single ttbar event with PU 200 and Fatras and ODD

main

Number of measurements: 60163
Size of measurements: 22621288

changed

Number of measurements: 60163
Size of measurements: 7696227

22,6 MB -> 7,7 MB ~ factor 3

compared to the track edm

Track State memory statistics (averaged):
other:
               count: 17111.000000
               index:     0.91M
             parPred:     0.78M
             covPred:     4.70M
             parFilt:     0.78M
             covFilt:     4.70M
             parSmth:     0.78M
             covSmth:     4.67M
                meas:     0.00M
          measOffset:     0.07M
             measCov:     0.00M
       measCovOffset:     0.07M
                 jac:     4.70M
         sourceLinks:     0.39M
          projectors:     0.00M
               total:    22.55M
meas:
               count: 16938.00
               index:     0.90M
             parPred:     0.78M
             covPred:     4.65M
             parFilt:     0.78M
             covFilt:     4.65M
             parSmth:     0.76M
             covSmth:     4.58M
                meas:     0.28M
          measOffset:     0.06M
             measCov:     0.70M
       measCovOffset:     0.06M
                 jac:     4.65M
         sourceLinks:     0.78M
          projectors:     0.13M
               total:    23.77M

given

Number of tracks: 1321
Number of track states: 34049

it looks like we have to add them up. also this will not contain any track information but that only scales with 1321 which is not significant here.

~ 50 MB track edm
~ 22,6 MB measurement edm before
~ 7,7 MB measurement edm after

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.

A few comments, none of them critical. I think just a little bit of documentation would be good.

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 to me!

Copy link

sonarqubecloud bot commented Sep 4, 2024

@kodiakhq kodiakhq bot merged commit 8c91d41 into acts-project:main Sep 4, 2024
42 checks passed
@github-actions github-actions bot removed the automerge label Sep 4, 2024
@andiwand andiwand deleted the ex-dense-measurement-container branch September 4, 2024 20:28
@paulgessinger paulgessinger modified the milestones: next, v37.0.0 Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants