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

Use SiStripApvSimulationParameters from EventSetup #4

Conversation

pieterdavid
Copy link

Digitizer part of #3, to be combined with cms-sw#27941

Copy link

@mmusich mmusich left a comment

Choose a reason for hiding this comment

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

Just a couple of aesthetical comments

@EmyrClement
Copy link
Owner

@pieterdavid thanks for this! I will test these two PRs together (along with the more cosmetic changes for my original PR) to check I get the same output in the validation plots I produce.

@EmyrClement
Copy link
Owner

I ran with these changes and with cms-sw#27941, and I get hte same outcome. You can see the cluster multiplicity vs PU here, so it looks like the APV baseline distributions are implemented and sampled correctly with the new code.

I am happy to merge this now - any objections?

@pieterdavid
Copy link
Author

Great to hear that, thanks for checking! I don't have anything else for now, let's see what the next step of review of the CMSSW PRs brings (the only thing that is still pending, and which probably needs something on the C++ side, is the fact that we want this for only about half of the 2016 statistics, but we can first collect some feedback on the current implementation).

@mmusich
Copy link

mmusich commented Sep 9, 2019

the only thing that is still pending, and which probably needs something on the C++ side, is the fact that we want this for only about half of the 2016 statistics.

One needs to calculate the actual luminosity affected by the issue and use that to throw a dice in order to use the feature lumi_affected/total_lumi cases. I would configure that number either from the DB object into another member maybe just in the python configuration.

@EmyrClement EmyrClement merged commit bef7dbf into EmyrClement:APVSimulation_PR_master Sep 9, 2019
EmyrClement pushed a commit that referenced this pull request Jun 5, 2020
…pre4

Ian l1 tk integration cmssw 11 1 0 pre4
EmyrClement pushed a commit that referenced this pull request Feb 5, 2021
Fix read i/o rules for backward accessibility of scouting dataset
EmyrClement pushed a commit that referenced this pull request Feb 5, 2021
EmyrClement pushed a commit that referenced this pull request May 16, 2023
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.

3 participants