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

EtaBToJpsiJpsi gen fragment at 14 TeV for release validation #33386

Merged
merged 5 commits into from Apr 30, 2021

Conversation

vjmastra
Copy link
Contributor

@vjmastra vjmastra commented Apr 9, 2021

PR description:

EtaB to JpsiJpsi to 4muon gen fragment for release validation
targeting BPH multi-lepton trigger paths, not covered for the time being.
The etaB is modeled with a BSM H0 (as done in previous versions of the fragment)

PR validation:

The measured filter efficiency on a generated sample of 5k events is 35%

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33386/21999

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 9, 2021

A new Pull Request was created by @vjmastra (Vincenzo) for master.

It involves the following packages:

Configuration/Generator
Configuration/PyReleaseValidation

@SiewYan, @mkirsano, @jordan-martins, @chayanit, @wajidalikhan, @kpedro88, @cmsbuild, @GurpreetSinghChahal, @srimanob, @agrohsje, @alberto-sanchez can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @makortel, @fabiocos, @slomeo this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@kpedro88
Copy link
Contributor

kpedro88 commented Apr 9, 2021

this will definitely conflict with #33324 (just so everyone is aware)

@chayanit
Copy link

Is it possible that extra HLT requests come together in the same PR? This is confusing...
@sharmaaash, @vjmastra

@srimanob
Copy link
Contributor

Hi @vjmastra
I have to admit that I confuse the goal of this PR. If these GEN fragments are not needed for any workflows in PyReleaseValidation, it should go to https://github.com/cms-sw/genproductions. We should not add something that we don't need to CMSSW. With RelVals machine (not runTheMatrix), PdmV can pick up any configs.

@vjmastra
Copy link
Contributor Author

Hi @vjmastra
I have to admit that I confuse the goal of this PR. If these GEN fragments are not needed for any workflows in PyReleaseValidation, it should go to https://github.com/cms-sw/genproductions. We should not add something that we don't need to CMSSW. With RelVals machine (not runTheMatrix), PdmV can pick up any configs.

Hi @srimanob, the GEN fragment I uploaded is needed for the release validation (as described here https://its.cern.ch/jira/browse/PDMVRELVALS-115).

Hi @chayanit, from the comments on Jira I understand that it is ok to upload the fragment,
while the change (1 line addition) in upgradeWorkflowComponents.py is not.
Do you need me to take action on this or it can be handled by the reviewers?

@srimanob
Copy link
Contributor

Ah, my false sorry. You define the fragment in upgradeWorkflowComponents.py also.

@srimanob
Copy link
Contributor

My comment is then the filename should not contain "_forSTEAM". It is GEN fragment file that can be used for anything.


generator.PythiaParameters.processParameters.extend(EvtGenExtraParticles)

etafilter = cms.EDFilter("PythiaFilter",

Choose a reason for hiding this comment

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

why is this called etafilter?

Copy link
Contributor Author

@vjmastra vjmastra Apr 13, 2021

Choose a reason for hiding this comment

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

I did as in previous version of the fragment available in CMSSW (

)

The BSM Higgs (pdgid=35) with mass forced to the same as etaB is used instead of an etaB.
The filter requires its presence.

'443:onMode = off',
'443:onIfMatch 13 -13',
############# Fix to mass of etaB (9.4 GeV) #############
'35:mMin = 0',

Choose a reason for hiding this comment

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

what is the purpose of min and max if you set 0 width ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left the properties as in the previous version of the fragment

The definition of the mass range should have no effect as width is set to 0

@agrohsje
Copy link

agrohsje commented Apr 20, 2021

I have a small comment on the name. If we want to follow the dataset convention it should be TuneCP5_14TeV_pythia8. Sometimes we indicate ptyhia8 in fragments, sometimes not. Clearly having it makes it easier to understand what is hidden in the file. Not sure if filter info should be added. The name includes most of it. No strong opinion.

@vjmastra
Copy link
Contributor Author

I have a small comment on the name. If we want to follow the dataset convention it should be TuneCP5_14TeV_pythia8. Sometimes we indicate ptyhia8 in fragments, sometimes not. Clearly having it makes it easier to understand what is hidden in the file. Not sure if filter info should be added. The name includes most of it. No strong opinion.

I couldn't see this kind of naming under Configuration/Generator/python ...
please let me know how to act on this and if other actions are needed, thanks

@agrohsje
Copy link

Maybe you can just add _pythia8, so EtaBToJpsiJpsi_14TeV_TuneCP5_pythia8_cfi.py

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33386/22370

  • This PR adds an extra 28KB to repository

  • Found files with invalid states:

    • Configuration/Generator/python/EtaBToJpsiJpsi_forSTEAM_14TeV_TuneCP5_cfi.py:
    • Configuration/Generator/python/EtaBToJpsiJpsi_14TeV_TuneCP5_cfi.py:

@cmsbuild
Copy link
Contributor

Pull request #33386 was updated. @SiewYan, @mkirsano, @cmsbuild, @GurpreetSinghChahal, @agrohsje, @alberto-sanchez can you please check and sign again.

@agrohsje
Copy link

+generators

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b2201d/14714/summary.html
COMMIT: f804491
CMSSW: CMSSW_12_0_X_2021-04-29-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33386/14714/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2662700
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2662675
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@qliphy
Copy link
Contributor

qliphy commented Apr 30, 2021

+1

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

7 participants