-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
New_samples (TTbarLepton_14 and QQH1252T_14) added without updating 'upgradeWorkflowComponents' file #33409
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33409/22046
|
A new Pull Request was created by @sharmaaash (Ashish Sharma) for master. It involves the following packages: Configuration/Generator @SiewYan, @mkirsano, @cmsbuild, @GurpreetSinghChahal, @agrohsje, @alberto-sanchez can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b4fe4d/14190/summary.html Comparison SummarySummary:
|
pythia8CP5SettingsBlock, | ||
processParameters = cms.vstring( | ||
'Top:gg2ttbar = on ', | ||
'Top:gg2ttbar = on ', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the reason to have this line twice? is this supposed to be qqbar ?
pythiaHepMCVerbosity = cms.untracked.bool(False), | ||
# put here the cross section of your process (in pb) | ||
crossSection = cms.untracked.double(0.388), | ||
maxEventsToPrint = cms.untracked.int32(3), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to set to 0 for relval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean cross section or which parameter to set to zero?
Please see my comments on the fragments. About names. Why is it called TTbarLepton ? Instead of QQH1252T I would propose QQToHToTauTau_mh125 or so. The general ending could be TuneCP5_14TeV_pythia8, just to make it easier to understand what is encoded in the fragment. |
Pull request #33409 was updated. @SiewYan, @mkirsano, @cmsbuild, @GurpreetSinghChahal, @agrohsje, @alberto-sanchez can you please check and sign again. |
@sharmaaash now the top is called higgstotautau. I think you were misreading my comment. :-) |
…epton_mt172p5_TuneCP5_14TeV_pythia8_cfi.py
…4TeV_pythia8_cfi.py
I made changes. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33409/22360
|
Pull request #33409 was updated. @SiewYan, @mkirsano, @cmsbuild, @GurpreetSinghChahal, @agrohsje, @alberto-sanchez can you please check and sign again. |
+generators |
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) |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b4fe4d/14684/summary.html Comparison SummarySummary:
|
+1 |
Configuration/Generator/python/QQToHToTauTau_mh125_TuneCP5_14TeV_pythia8_cfi.py
Show resolved
Hide resolved
@@ -0,0 +1,29 @@ | |||
import FWCore.ParameterSet.Config as cms | |||
from Configuration.Generator.Pythia8CommonSettings_cfi import * | |||
from Configuration.Generator.Pythia8CP5Settings_cfi import * |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
we noticed that this import has a wrong path and failed in production. It should be fixed soon.
from Configuration.Generator.MCTunes2017.PythiaCP5Settings_cfi import *
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So do I need to create another PR with the following changes with the replacement ---> from Configuration.Generator.MCTunes2017.PythiaCP5Settings_cfi import *
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So do I need to create another PR with the following changes with the replacement ---> from Configuration.Generator.MCTunes2017.PythiaCP5Settings_cfi import *
?
Yes, please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made following changes and recreate PR:
#33700
@chayanit You are right. I overlooked this when giving my ok. @qliphy @silviodonato is there a way to include technical checks for python scripts? Or do we always have to ask the person who makes the PR if the script was tested. For more complicated once, like H7 it would be even more likely that such mistakes get not spot when checking by eye. Do we need to rely on local testing or are there alternatives? |
@agrohsje if you can run the fragment using runTheMatrix.py, we can test the fragment in the PR tests. Otherwise we have to rely on the author or GEN group private test. |
Hi @silviodonato. Ok. So if not mentioned we will double-check. Thanks! |
PR description:
Samples "TTbarLepton_14 and QQH1252T_14" added.
No additional changes in 'upgradeWorkflowComponents.py' file
PR validation:
if this PR is a backport please specify the original PR and why you need to backport that PR:
Before submitting your pull requests, make sure you followed this checklist: