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: update Open Data Detector to v4.0.1 #2992

Merged
merged 6 commits into from
Mar 14, 2024

Conversation

asalzburger
Copy link
Contributor

@asalzburger asalzburger commented Feb 27, 2024

This PR updates the open data detector to v4.0.1, which will enable it to built in Gen1-style and Gen2-style geometry.

Small performance changes are expected, since v4.0.1 also contains a few tiny geometry fixes.

This PR increases the 'examples' run time from 11 to 15 because it will run Geant4 material recording also in the Calorimeter (which is on by default) in the ODD ...

@asalzburger asalzburger added this to the next milestone Feb 27, 2024
Copy link

codecov bot commented Feb 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.82%. Comparing base (682d208) to head (bf0bfa3).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2992   +/-   ##
=======================================
  Coverage   48.82%   48.82%           
=======================================
  Files         491      491           
  Lines       28886    28886           
  Branches    13705    13705           
=======================================
  Hits        14104    14104           
  Misses       4954     4954           
  Partials     9828     9828           

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

@andiwand
Copy link
Contributor

Something related to time seems to be broken

image

that might explain the other differences

@andiwand
Copy link
Contributor

@asalzburger asalzburger changed the title feat: update Open Data Detector to v4.0.0 feat: update Open Data Detector to v4.0.1 Mar 12, 2024
andiwand
andiwand previously approved these changes Mar 12, 2024
Copy link
Contributor

@andiwand andiwand left a comment

Choose a reason for hiding this comment

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

Output looks much more sane now. To me it looks like we shuffle around some outliers which triggers all the physmon failure.

No objections from my side.

@github-actions github-actions bot added Infrastructure Changes to build tools, continous integration, ... Changes Performance labels Mar 13, 2024
@asalzburger
Copy link
Contributor Author

@andiwand @paulgessinger Any thoughts on the example_test runtime increase due to the calorimeter being built per default?

I tried to introduce <xlst:choose> condition paired with environment variables to make it switchable without changing the xml file, but could not get it to run with the DD4hep xml schema. For the moment, I guess we have to live with it, then we could change the ODD xml setup to DD4hep style.

<your application> OdenDataDetector.xml OpenDataTracker.xml OpenDataCalorimeter.xml 

@paulgessinger
Copy link
Member

@asalzburger how much are we talking here? I wouldn't have expected this to be large increase on the ACTS side, is this driven by DD4hep taking longer to construct?

@andiwand
Copy link
Contributor

What is the runtime increase?

Alternatively we could have different ODD xml root files I guess? One with and one without the calo.
Or we split it completely like you proposed.

@asalzburger
Copy link
Contributor Author

@asalzburger how much are we talking here? I wouldn't have expected this to be large increase on the ACTS side, is this driven by DD4hep taking longer to construct?

Testing goes from 11min to 15mins, and it's practically driven by the material_recording and genat4 simulation now seeing a calorimeter.

@asalzburger
Copy link
Contributor Author

What is the runtime increase?

Alternatively we could have different ODD xml root files I guess? One with and one without the calo. Or we split it completely like you proposed.

Yeah, I guess we'd have to do some "OpenDataDetectorTrackerOnly.xml"

@asalzburger
Copy link
Contributor Author

@andiwand - just to make sure, as you merged in AMVF changes int he meantime, the performance reference needs to be updated - looks all good to me, but if you could have another look, I will update the file.

@andiwand
Copy link
Contributor

this is the usual problem with concurrent reference file changes. there is one more ref change in the pipeline #3034. afterwards we can proceed with this one

to me 11->15 min sounds not too bad. also these numbers fluctuate quite a bit depending on our load

@github-actions github-actions bot added the Component - Examples Affects the Examples module label Mar 14, 2024
@andiwand andiwand merged commit 4f33d42 into acts-project:main Mar 14, 2024
54 checks passed
dimitra97 pushed a commit to dimitra97/acts that referenced this pull request Mar 19, 2024
This PR updates the open data detector to v4.0.1, which will enable it
to built in Gen1-style and Gen2-style geometry.

Small performance changes are expected, since v4.0.1 also contains a few
tiny geometry fixes.

This PR increases the 'examples' run time from 11 to 15 because it will
run Geant4 material recording also in the Calorimeter (which is on by
default) in the ODD ...

---------

Co-authored-by: Andreas Stefl <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
@paulgessinger paulgessinger modified the milestones: next, v33.1.0 Mar 26, 2024
EleniXoch pushed a commit to EleniXoch/acts that referenced this pull request May 6, 2024
This PR updates the open data detector to v4.0.1, which will enable it
to built in Gen1-style and Gen2-style geometry.

Small performance changes are expected, since v4.0.1 also contains a few
tiny geometry fixes.

This PR increases the 'examples' run time from 11 to 15 because it will
run Geant4 material recording also in the Calorimeter (which is on by
default) in the ODD ...

---------

Co-authored-by: Andreas Stefl <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
asalzburger added a commit to asalzburger/acts that referenced this pull request May 21, 2024
This PR updates the open data detector to v4.0.1, which will enable it
to built in Gen1-style and Gen2-style geometry.

Small performance changes are expected, since v4.0.1 also contains a few
tiny geometry fixes.

This PR increases the 'examples' run time from 11 to 15 because it will
run Geant4 material recording also in the Calorimeter (which is on by
default) in the ODD ...

---------

Co-authored-by: Andreas Stefl <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes Performance Component - Examples Affects the Examples module Infrastructure Changes to build tools, continous integration, ...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants