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

Adding GEN fragment (GluGluTo2Jets_ExHume) specific for PPS wf testing #32765

Closed
wants to merge 8 commits into from

Conversation

mundim
Copy link
Contributor

@mundim mundim commented Jan 28, 2021

PR description:

Add a GEN fragment (ExHume generator, gluongluon -> 2 Jets, 14 TeV, central mass between 300 and 2000 GeV) to produce protons inside PPS acceptance to allow it to be properly tested.

PR validation:

scram b runtests
scram b code-checks
scram b code-format

ran without any issues.

standard runTheMatrix --jobsReports -l limited --ibeos
produced no error

runTheMatrix.py --what upgrade -l 11725.0,11925.0,11925.98,11925.99,12125.0,12325.0,12325.98,12325.99 -t 4 --command='-n 5

where the wf are the ones in which GluGluTo2Jets is present (as advised in #32352), produced no error for the ones with NO PU, but it was reported missing files for the PU ones

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32765/20923

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mundim 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

@srimanob
Copy link
Contributor

test parameters:

  • workflows = 11725.0,11925.0,11925.98,11925.99,12125.0,12325.0,12325.98,12325.99

@srimanob
Copy link
Contributor

Please test

@mundim
Copy link
Contributor Author

mundim commented Jan 28, 2021

the wf 12325.0, 11925.0, 12325.98, 11925.98, 12325.99 and 11925.99 (the ones with PU) failed in my area because of a missing secondary file. I hope it will not happen in the official testing area.

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-dfda86/12591/summary.html
COMMIT: 4425c67
CMSSW: CMSSW_11_3_X_2021-01-28-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/32765/12591/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

@mundim
Copy link
Contributor Author

mundim commented Jan 28, 2021

@chayanit sorry, I forgot to mention you here.

@@ -1288,4 +1288,5 @@ def __init__(self, howMuch, dataset):
('TenTau_E_15_500_Eta3p1_pythia8_cfi', UpgradeFragment(Kby(9,100),'TenTau_15_500_Eta3p1')),
('QCD_Pt_1800_2400_14TeV_TuneCP5_cfi', UpgradeFragment(Kby(9,50), 'QCD_Pt_1800_2400_14')),
('DisplacedSUSY_stopToBottom_M_800_500mm_TuneCP5_14TeV_pythia8_cff', UpgradeFragment(Kby(9,50),'DisplacedSUSY_stopToB_M_800_500mm_14')),

Choose a reason for hiding this comment

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

Would you mind to help me also update this line so I don't need to open new PR?
('DisplacedSUSY_stopToBottom_M_800_500mm_TuneCP5_14TeV_pythia8_cff', UpgradeFragment(Kby(9,50),'DisplacedSUSY_14TeV')),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

@@ -1288,4 +1288,5 @@ def __init__(self, howMuch, dataset):
('TenTau_E_15_500_Eta3p1_pythia8_cfi', UpgradeFragment(Kby(9,100),'TenTau_15_500_Eta3p1')),
('QCD_Pt_1800_2400_14TeV_TuneCP5_cfi', UpgradeFragment(Kby(9,50), 'QCD_Pt_1800_2400_14')),
('DisplacedSUSY_stopToBottom_M_800_500mm_TuneCP5_14TeV_pythia8_cff', UpgradeFragment(Kby(9,50),'DisplacedSUSY_stopToB_M_800_500mm_14')),
('GluGluTo2Jets_M_300_2000_14TeV_Exhume_cff',UpgradeFragment(Kby(9,100),'GluGluTo2Jets_M_300_2000_14TeV')),

Choose a reason for hiding this comment

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

I'm afraid the string is a bit too long. I would request to change to ('GluGluTo2Jets_M_300_2000_14TeV_Exhume_cff',UpgradeFragment(Kby(9,100),'GluGluTo2Jets_14TeV')),

@chayanit
Copy link

please test
@srimanob I don't think we can ask bot to test new workflows.

@chayanit
Copy link

@chayanit sorry, I forgot to mention you here.

no need. I'm included as L2 PdmV to assign this PR already. Could you commit changes as I requested? Thanks.

@chayanit
Copy link

chayanit commented Feb 17, 2021

Hi @chayanit. I was trying to understand this... I'm not sure about your question. The wf ID I took from runTheMatrix.py --what upgrade --showMatrix | grep -i GluGluTo2Jets_M | grep 2021. But it seems the PU wf is trying to get a MinBias from 2011, in which certainly, PPS was not present and I imagine a mixing will fail, right?

@mundim Can you show me what you got after run runTheMatrix.py --what upgrade --showMatrix | grep -i GluGluTo2Jets_M | grep 2021 only for these 4 workflows 11725.0, 11925.0 and 12125.0, 12325.0?

@mundim
Copy link
Contributor Author

mundim commented Feb 17, 2021

Hi. this is the output:

11725.0 2021+GluGluTo2Jets_M_300_2000_14TeV_Exhume_GenSim+Digi+Reco+HARVEST+ALCA
11925.0 2021PU+GluGluTo2Jets_M_300_2000_14TeV_Exhume_GenSim+DigiPU+RecoPU+HARVESTPU+Nano
12125.0 2021Design+GluGluTo2Jets_M_300_2000_14TeV_Exhume_GenSim+Digi+Reco+HARVEST
12325.0 2021DesignPU+GluGluTo2Jets_M_300_2000_14TeV_Exhume_GenSim+DigiPU+RecoPU+HARVESTPU

@chayanit
Copy link

Hi. this is the output:

11725.0 2021+GluGluTo2Jets_M_300_2000_14TeV_Exhume_GenSim+Digi+Reco+HARVEST+ALCA
11925.0 2021PU+GluGluTo2Jets_M_300_2000_14TeV_Exhume_GenSim+DigiPU+RecoPU+HARVESTPU+Nano
12125.0 2021Design+GluGluTo2Jets_M_300_2000_14TeV_Exhume_GenSim+Digi+Reco+HARVEST
12325.0 2021DesignPU+GluGluTo2Jets_M_300_2000_14TeV_Exhume_GenSim+DigiPU+RecoPU+HARVESTPU

OK. We should select 11725.0 (noPU) and 11925.0 (PU) as they are standard scenario and not the Design ones. Can you update in relval_2017 file to these two workflows instead? @mundim

@mundim
Copy link
Contributor Author

mundim commented Feb 18, 2021

I solved the conflict about relval_2017, but I'm not sure about the other 2 files (I used the latest IB as the base).

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32765/21176

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@chayanit
Copy link

Hello @mundim , @qliphy , @silviodonato any problem with this PR? It seems to conflict with others. How should we fix and move this PR?

@silviodonato
Copy link
Contributor

After having merged #32969 and #32868 everything should be already merged in master.
@mundim could you check all the code is included in https://github.com/cms-sw/cmssw ?

@mundim
Copy link
Contributor Author

mundim commented Feb 23, 2021

After having merged #32969 and #32868 everything should be already merged in master.
@mundim could you check all the code is included in https://github.com/cms-sw/cmssw ?

Some how the proposed change in realval_2017.py have been ignored as well as some corrections in the SM parameters in the generator fragment.

@qliphy
Copy link
Contributor

qliphy commented Feb 24, 2021

After having merged #32969 and #32868 everything should be already merged in master.
@mundim could you check all the code is included in https://github.com/cms-sw/cmssw ?

Some how the proposed change in realval_2017.py have been ignored as well as some corrections in the SM parameters in the generator fragment.

@mundim As mentioned in #32969 (comment) you can open a new PR to add all missing fixes. Thanks!

@silviodonato
Copy link
Contributor

@mundim
I squashed the commits of this PR and fixed the conflict with master.
Does https://github.com/cms-sw/cmssw/compare/master...silviodonato:PPSRelValGenFragment_reb?expand=1 look ok ?

@mundim
Copy link
Contributor Author

mundim commented Feb 24, 2021

@mundim
I squashed the commits of this PR and fixed the conflict with master.
Does https://github.com/cms-sw/cmssw/compare/master...silviodonato:PPSRelValGenFragment_reb?expand=1 look ok ?

It is ok with the gen fragment, but in the relval_2017.py file, lines 33 and 60 got added instead of changed, they are not duplicated.

@silviodonato
Copy link
Contributor

the

@mundim
I squashed the commits of this PR and fixed the conflict with master.
Does https://github.com/cms-sw/cmssw/compare/master...silviodonato:PPSRelValGenFragment_reb?expand=1 look ok ?

It is ok with the gen fragment, but in the relval_2017.py file, lines 33 and 60 got added instead of changed, they are not duplicated.

@mundim Right, thanks a lot. I've fixed it. Will you open the PR or shall I do it?

@mundim
Copy link
Contributor Author

mundim commented Feb 24, 2021

the

@mundim
I squashed the commits of this PR and fixed the conflict with master.
Does https://github.com/cms-sw/cmssw/compare/master...silviodonato:PPSRelValGenFragment_reb?expand=1 look ok ?

It is ok with the gen fragment, but in the relval_2017.py file, lines 33 and 60 got added instead of changed, they are not duplicated.

@mundim Right, thanks a lot. I've fixed it. Will you open the PR or shall I do it?

Done already. thank you very much.

@silviodonato
Copy link
Contributor

moved to #32983

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

9 participants