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

ci: Pull more config into physmon script, extend PG eta range to +-4 #1320

Merged
merged 4 commits into from
Jul 12, 2022

Conversation

paulgessinger
Copy link
Member

I think for the physmon jobs, given the ODD, it makes little sense to only test particle up to $|\eta| < \pm2$. This PR adjusts physmon.py to include the particle gun configuration so I can customize it to produce particles up to $|\eta| < \pm 4$.

@paulgessinger paulgessinger added this to the next milestone Jul 11, 2022
@paulgessinger
Copy link
Member Author

Clearly this has physmon failures. To me the changes look as expected. I'll update the references once the CI runs through.

@codecov
Copy link

codecov bot commented Jul 11, 2022

Codecov Report

Merging #1320 (8e26506) into main (66b16ef) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1320   +/-   ##
=======================================
  Coverage   47.42%   47.42%           
=======================================
  Files         375      375           
  Lines       19788    19788           
  Branches     9287     9287           
=======================================
  Hits         9385     9385           
  Misses       4021     4021           
  Partials     6382     6382           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@paulgessinger
Copy link
Member Author

I had a look at the physmon outputs. They're basically as expected.

CKF Seeded

grafik
Not entirely sure why this looks so asymmetric.

grafik
grafik
Bit of an odd feature in the tracking efficiency in $\eta$. Since this doesn't show up in the other "seeding" modes, it might be some feature of the seeding?

CKF truth estimated

grafik

CKF truth smeared

grafik

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.

plots look mostly reasonable IMO

the on with the holes looks a bit weird but that might be due to statistical fluctuations? the error bars are quite big. we could check with a different eta distribution, a different seed or more samples. but I don't think it is necessary

@paulgessinger
Copy link
Member Author

I updated the references. I realized now that it's actually pretty hard to put in the correct reference commit SHA, because we squash and merge. No idea if that can be improved.

@andiwand
Copy link
Contributor

I updated the references. I realized now that it's actually pretty hard to put in the correct reference commit SHA, because we squash and merge. No idea if that can be improved.

yeah I think it is almost impossible. shouldn't be the git history be enough?

@paulgessinger
Copy link
Member Author

Probably yeah.

@paulgessinger
Copy link
Member Author

I think this is good now, @andiwand.

@kodiakhq kodiakhq bot merged commit dfd644d into acts-project:main Jul 12, 2022
@paulgessinger paulgessinger modified the milestones: next, v19.5.0 Jul 25, 2022
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.

2 participants