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

Fixed bug. Missing edm::InputTag src default value in fillDescriptions #41372

Merged
merged 1 commit into from May 2, 2023

Conversation

AWildridge
Copy link
Contributor

PR description:

There is currently a bug that makes the usage of the LHEGenericMassFilter unusable. This is because the fillDescriptions method was missing the default value for the edm::InputTag src constructor parameter. This PR fixes this bug.

PR validation:

I ran
scram b runtests
and
runTheMatrix.py -l limited -i all --ibeos
No new code needs to be added for unit tests, test configurations, additions, or updates to the runTheMatrix test workflows as far as I am concerned.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41372/35226

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @AWildridge (AJ Wildridge) for master.

It involves the following packages:

  • GeneratorInterface/GenFilters (generators)

@SiewYan, @mkirsano, @Saptaparna, @cmsbuild, @alberto-sanchez, @menglu21, @GurpreetSinghChahal can you please review it and eventually sign? Thanks.
@alberto-sanchez, @mkirsano this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@perrotta
Copy link
Contributor

please test

@perrotta
Copy link
Contributor

type bugfix

@perrotta
Copy link
Contributor

@AWildridge it is strange that bug: it looks like that the code was never actually tested during and after the merge of #39459
Could you please make sure that the filter can run after this bug fix, and post here some result of that run, so that we are sure that there is not anything else missing before we merge it?

@AWildridge
Copy link
Contributor Author

AWildridge commented Apr 19, 2023

@AWildridge it is strange that bug: it looks like that the code was never actually tested during and after the merge of #39459 Could you please make sure that the filter can run after this bug fix, and post here some result of that run, so that we are sure that there is not anything else missing before we merge it?

Yes I did not think adding default values would break the code so that is why it was not tested. I did not properly understand what all entailed in the addition of the fillDescriptions method.

I've performed a test generating the Toponium sample as mentioned in the prior PR. This is the fragment that I use:

import FWCore.ParameterSet.Config as cms

externalLHEProducer = cms.EDProducer("ExternalLHEProducer",
    args = cms.vstring('/depot/cms/top/awildrid/Toponium/CMSSW_10_6_19/src/EtaTTo2L2Nu_0j_madgraph_4f_NNPDF31nnlo_13TeV_slc7_amd64_gcc700_CMSSW_10_6_19_tarball.tar.xz'),
    nEvents = cms.untracked.uint32(10),
    numberOfParameters = cms.uint32(1),
    outputFile = cms.string('cmsgrid_final.lhe'),
    scriptName = cms.FileInPath('GeneratorInterface/LHEInterface/data/run_generic_tarball_cvmfs.sh')
)
# link to cards: 
# https://github.com/cms-sw/genproductions/tree/master/bin/MadGraph5_aMCatNLO/cards/production/13TeV/EtaTTo2L2Nu_and_Semileptonic_0j_4f_NNPDF31nnlo_13TeV/dilepton/


import FWCore.ParameterSet.Config as cms
from Configuration.Generator.Pythia8CommonSettings_cfi import *
from Configuration.Generator.MCTunes2017.PythiaCP5Settings_cfi import *
from Configuration.Generator.PSweightsPythia.PythiaPSweightsSettings_cfi import *

generator = cms.EDFilter("Pythia8HadronizerFilter",
                         maxEventsToPrint = cms.untracked.int32(1),
                         pythiaPylistVerbosity = cms.untracked.int32(1),
                         filterEfficiency = cms.untracked.double(1.0),
                         pythiaHepMCVerbosity = cms.untracked.bool(False),
                         comEnergy = cms.double(13000.),
                         PythiaParameters = cms.PSet(
                             pythia8CommonSettingsBlock,
                             pythia8CP5SettingsBlock,
                             #pythia8PSweightsSettingsBlock,
                             parameterSets = cms.vstring('pythia8CommonSettings',
                                                         'pythia8CP5Settings',
                                                         #'pythia8PSweightsSettings',
                             )
                         )
)

etatMassFilter = cms.EDFilter("LHEGenericMassFilter",
                              NumRequired = cms.int32(4),
                              ParticleID = cms.vint32(24, 5, -24, -5),
                              MinMass = cms.double(337.),
                              MaxMass = cms.double(349.),
                              RequiredOutgoingStatus = cms.bool(False),
                              src = cms.InputTag("externalLHEProducer")
)

ProductionFilterSequence = cms.Sequence(etatMassFilter + generator)

I used this command of cmsDriver to generate the config file:
cmsDriver.py GeneratorEventFilter/LHEFilter/python/fragment_0j_LHEGenericMassFilter.py --fileout [file:nanogen.root]file:nanogen.root --mc --eventcontent NANOAODSIM --datatier NANOAOD --conditions auto:mc --step LHE,GEN,NANOGEN --python_filename etat_nanogen_LHEGenericMassFilter.py --no_exec -n 5000

I then used the command:
cmsRun etat_nanogen_LHEGenericMassFilter.py to generate the following output (only showing end results since its lots of lines):

Overall cross-section summary 
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Process		xsec_before [pb]		passed	nposw	nnegw	tried	nposw	nnegw 	xsec_match [pb]			accepted [%]	 event_eff [%]
0		5.227e-04 +/- 2.020e-06		1890	1890	0	1890	1890	0	5.227e-04 +/- 2.020e-06		100.0 +/- 0.0	100.0 +/- 0.0
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------- 
Total		5.227e-04 +/- 2.020e-06		1890	1890	0	1890	1890	0	5.227e-04 +/- 2.020e-06		100.0 +/- 0.0	100.0 +/- 0.0
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Before matching: total cross section = 5.227e-04 +- 2.020e-06 pb
After matching: total cross section = 5.227e-04 +- 2.020e-06 pb
Matching efficiency = 1.0 +/- 0.0   [TO BE USED IN MCM]
Filter efficiency (taking into account weights)= (0.987858) / (0.987858) = 1.000e+00 +- 0.000e+00
Filter efficiency (event-level)= (1890) / (1890) = 1.000e+00 +- 0.000e+00    [TO BE USED IN MCM]

After filter: final cross section = 5.227e-04 +- 2.020e-06 pb
After filter: final fraction of events with negative weights = 0.000e+00 +- 0.000e+00
After filter: final equivalent lumi for 1M events (1/fb) = 1.913e+06 +- 7.639e+03

The MCnet usage guidelines apply to Rivet: see http://www.montecarlonet.org/GUIDELINES
Please acknowledge Rivet in results made using it, and cite https://arxiv.org/abs/1912.05451
Thanks for using LHAPDF 6.4.0. Please make sure to cite the paper:
  Eur.Phys.J. C75 (2015) 3, 132  (http://arxiv.org/abs/1412.7420)

Please ignore the cross section as we need to reweight this sample.

I then opened that file and grabbed the mass for the toponium particles with status 62 and I get this distribution (did not use weights):
image

Please let me know if you would like me to run further or different tests.

@perrotta
Copy link
Contributor

Thank you @AWildridge
For what the release is concerrned, I just wanted to be sure that there were no other errors hidden in the code: if it runs without crashing, that's enough for me. I'll let evaluate the physics content to the @cms-sw/generators-l2 reviewers

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-31c6ea/32036/summary.html
COMMIT: e791e12
CMSSW: CMSSW_13_1_X_2023-04-18-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/41372/32036/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 3 lines from the logs
  • Reco comparison results: 16 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3459877
  • DQMHistoTests: Total failures: 95
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3459760
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 207 log files, 159 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@AWildridge
Copy link
Contributor Author

Thank you @AWildridge For what the release is concerrned, I just wanted to be sure that there were no other errors hidden in the code: if it runs without crashing, that's enough for me. I'll let evaluate the physics content to the @cms-sw/generators-l2 reviewers

Sounds good. Regarding the physics content, this is probably a more accurate plot since the filtering was performed on the W+W-bb~ invariant mass. This is the W+W-bb~ invariant mass where the W's and b's were selected if they had status 22 or 23. The total number of entries in this histogram is 1890 which matches the output above.

image

@AWildridge
Copy link
Contributor Author

@Saptaparna @menglu21 Just a friendly reminder regarding this pull request. I believe you are the L2s that Andrea mentioned, sorry if I am pinging the wrong people.

@menglu21
Copy link
Contributor

menglu21 commented May 2, 2023

Hi, sorry for overlooking this PR, may I ask that you have checked that without this fix this filter can't obtain the plot, is that right

@AWildridge
Copy link
Contributor Author

AWildridge commented May 2, 2023

No worries! Yes, I get the below error when I try running it with the src parameter in the fragment:

----- Begin Fatal Exception 18-Jan-2023 10:44:49 EST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=LHEGenericMassFilter label='etatMassFilter'
Exception Message:
Illegal parameter found in configuration.  The parameter is named:
 'src'
You could be trying to use a parameter name that is not
allowed for this plugin or it could be misspelled.
----- End Fatal Exception -------------------------------------------------

When I try removing the src part of the fragment I get:

An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=LHEGenericMassFilter label='etatMassFilter'
Exception Message:
MissingParameter: Parameter 'src' not found.
----- End Fatal Exception -------------------------------------------------```

@perrotta
Copy link
Contributor

perrotta commented May 2, 2023

+1

@perrotta
Copy link
Contributor

perrotta commented May 2, 2023

merge

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