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

Tune dependent reweighting #24174

Merged
merged 2 commits into from Sep 5, 2018
Merged

Conversation

zleba
Copy link

@zleba zleba commented Aug 2, 2018

Small change that allows having a different reweighting function for each tune.
We observed that at highPt the MC behaviour is very tune(PDF) dependent.
The generator fragment can currently contain
reweightGenEmp = cms.PSet(
tune = cms.string('CP5')
)
which will do MC reweighting suitable for CP5 tune.
Without the tune option fragment behaves as before (backward compatibility).

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2018

The code-checks are being triggered in jenkins.

@zleba zleba changed the base branch from master to CMSSW_9_3_X August 2, 2018 11:37
@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/29627/console Started: 2018/08/02 15:05

@alberto-sanchez
Copy link
Member

@zleba did you applied these changes in master, which PR was?

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2018

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 2, 2018

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24174/29627/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 25
  • DQMHistoTests: Total histograms compared: 2624448
  • DQMHistoTests: Total failures: 228
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2624043
  • DQMHistoTests: Total skipped: 177
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -24 KiB( 24 files compared)
  • Checked 105 log files, 8 edm output root files, 25 DQM output files

@zleba
Copy link
Author

zleba commented Aug 2, 2018

@alberto-sanchez The modified files are quite stable across version, so the same changes can be done on master.
Should I create new pull request for the master branch?
In my rep. is basically this commit
zleba@0b51a1c

In the end we want to use this feature for 2017 and 2018 MC production, so in some way to backport to 9_3_X and 10_X_X

@alberto-sanchez
Copy link
Member

Hi @zleba,
Yes, it has to go to master, and then to the other releases where you want to use, in this case
10_2_X, 9_3_X

@alberto-sanchez
Copy link
Member

backport #24179

@alberto-sanchez
Copy link
Member

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 3, 2018

This pull request is fully signed and it will be integrated in one of the next CMSSW_9_3_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_10_3_X is complete. This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos, @kpedro88 (and backports should be raised in the release meeting by the corresponding L2)

@fabiocos
Copy link
Contributor

fabiocos commented Sep 5, 2018

@alberto-sanchez following the discussion at the ORP, as far as I can see the previous behaviour is maintained unless the tune is CP5, which was not the default, am I correct?

@zleba
Copy link
Author

zleba commented Sep 5, 2018

Hi, yes, with the old MC fragments the behaviour is preserved.
For the Pythia 8 runs with the newest CP5 tune the default reweighting does not work good, so one can set tune equal 'CP5' and reweighting formula with different values of parameters will be used (appropriate for CP5).

There are 6 parameters of the reweighting formula, and in the code the values are hardcoded for CP5 tune and for the older CUETP8 tune (default), the other possibility would be to read the parameters from the fragment itself.
Currently, with any new Py8 tune we need to change the CMSSW code to add the proper reweighting parameters for the new tune.

@fabiocos
Copy link
Contributor

fabiocos commented Sep 5, 2018

@zleba thank you for the confirmation, as far as this backport is concerned it looks ok. But in general would not be advisable to have these parameters passed through a PSet? Having them hardcoded looks a fragile solution in general...

@fabiocos
Copy link
Contributor

fabiocos commented Sep 5, 2018

+1

@cmsbuild cmsbuild merged commit d43ee61 into cms-sw:CMSSW_9_3_X Sep 5, 2018
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.

None yet

4 participants