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

JME DQM implementation of Z/Gamma plus Jet monitoring for Run3 (14_1_X) #44411

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

esiam
Copy link
Contributor

@esiam esiam commented Mar 15, 2024

PR description:

Adding DQM code for Jets monitoring with Z/Gamma plus Jets analysis module. These changes were discussed with JME trigger group. @theochatzis , @slehti

You can find a first presentation for the motivation in TSG ( @missirol , Silvia Goy Lopez ) group meeting here

Besides, you can see some plots in this link These plots were produced with 14_0 CMSSW version by rerunning the hlt for a muon dataset for 60k events.

PR validation:

The implementations were tested by running DQM steps and see that the histos are produced. Besides they were tested by using the RunTheMatrix.py -l 141.044.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

This is not a backport. It may be backported in CMSSW_14_0_X if needed.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 15, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44411/39486

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @esiam for master.

It involves the following packages:

  • DQMOffline/Trigger (dqm)

@syuvivida, @tjavaid, @rvenditti, @cmsbuild, @antoniovagnerini, @nothingface0 can you please review it and eventually sign? Thanks.
@rociovilar, @cericeci, @HuguesBrun, @Fedespring, @jhgoh, @trocino, @missirol, @mtosi this is something you requested to watch as well.
@sextonkennedy, @rappoccio, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

@tjavaid
Copy link

tjavaid commented Mar 15, 2024

please test

Copy link
Contributor

@mmusich mmusich left a comment

Choose a reason for hiding this comment

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

from a cursory diagonal look.

unsigned index = triggerNames_.triggerIndex(pathName); // find in which index is that path
if (!(hltcfgIndex == index)) {
if (print)
std::cout << "Error in trigger index" << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

no cout in production code, please use MessageLogger.

if (index > 0 && index < triggerNames_.size() &&
triggerResults->accept(index)) { //trigger is accepted and index is valid
if (print)
std::cout << "Trigger accepted" << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

no cout in production code, please use MessageLogger.

std::vector<std::string> module_names = hltConfig_.moduleLabels(index); // (pathname) works too
if (!(module_size == module_names.size())) {
if (print)
std::cout << "ERROR IN MODULES COUNTING" << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

no cout in production code, please use MessageLogger.

}
if (module_size == 0) {
if (print)
std::cout << "no modules in this path ?!?!" << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

no cout in production code, please use MessageLogger.

edm::InputTag moduleFilter;
moduleFilter = edm::InputTag(moduleName, "", processName_);
if (print)
std::cout << moduleFilter << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

no cout in production code, please use MessageLogger.

const bool doDirectBalancevsReferencePt = true,
const bool bookDen = false);

void FillME(ObjME* a_me,
Copy link
Contributor

Choose a reason for hiding this comment

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

coding rule 2.8: from https://cms-sw.github.io/cms_coding_rules.html:

Start method names with lowercase, use upper case initials for following words,

Comment on lines 78 to 79
edm::EDGetTokenT<trigger::TriggerEvent> triggerEventObject_;
edm::EDGetTokenT<edm::TriggerResults> triggerResultsToken_;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
edm::EDGetTokenT<trigger::TriggerEvent> triggerEventObject_;
edm::EDGetTokenT<edm::TriggerResults> triggerResultsToken_;
const edm::EDGetTokenT<trigger::TriggerEvent> triggerEventObject_;
const edm::EDGetTokenT<edm::TriggerResults> triggerResultsToken_;

as they are in the initializer list.

const std::string pathName;
const std::string moduleName;
edm::InputTag jetInputTag_;
edm::EDGetTokenT<reco::PFJetCollection> jetToken_;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
edm::EDGetTokenT<reco::PFJetCollection> jetToken_;
const edm::EDGetTokenT<reco::PFJetCollection> jetToken_;

as it is in the initializer list.

edm::EDGetTokenT<edm::TriggerResults> triggerResultsToken_;
const std::string pathName;
const std::string moduleName;
edm::InputTag jetInputTag_;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
edm::InputTag jetInputTag_;

unused.

}
if (!passTrig) {
if (print)
std::cout << "Trigger did not pass" << std::endl; // skip event if trigger fails
Copy link
Contributor

Choose a reason for hiding this comment

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

no cout in production code, please use MessageLogger.

@mmusich
Copy link
Contributor

mmusich commented Mar 15, 2024

@esiam

This is not a backport. It may be backported in CMSSW_14_0_X if needed.

aren't you targeting 2024 pp data-taking? if yes, a backport will be needed.

from JetMETCorrections.Configuration.JetCorrectors_cff import *

ak4PFJetsPuppiCorrected = cms.EDProducer('CorrectedPFJetProducer',
src = cms.InputTag('ak4PFJetsPuppi'),
Copy link
Contributor

Choose a reason for hiding this comment

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

this product seems to be not always available, see e.g. failures in wf 136.7801 and 136.7803

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi! Thanks for the comments! I will change them. Yes, it targets 2024 pp data-taking, sorry I was not sure for the backport.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi! I want to ask since this happened because Puppi Jets were not available in the input sample, but if the producer were not in cff it would not give an error. This is something we should avoid in general when using corrections in DQM? What I did is use corrections on the fly. I rerun and took same distributions and also do the code checks.
Thanks, Eirini.

Copy link
Contributor

Choose a reason for hiding this comment

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

@esiam

I rerun and took same distributions and also do the code checks.

thank you for the updates.

I want to ask since this happened because Puppi Jets were not available in the input sample, but if the producer were not in cff it would not give an error. This is something we should avoid in general when using corrections in DQM?

I know very little of the underlying motivation behind this PR, so I don't feel qualified to comment if you should "in general" avoid these corrections.
Allow me to point out the following facts:

In both workflows the DQM sequence run is DQM:offlineHLTSourceOnAOD which is defined here

offlineHLTSourceOnAOD = cms.Sequence(
dqmEnvHLT
* dqmHLTFiltersDQMonitor
* lumiMonitorHLTsequence
* muonFullOfflineDQM
* HLTTauDQMOffline
* hltInclusiveVBFSource
* higPhotonJetHLTOfflineSource # plots are filled, but I'm not sure who is really looking at them and what you can get from them ... good candidates to be moved in offlineHLTSourceOnAODextra
# eventshapeDQMSequence * ## OBSOLETE !!!! (looks for HLT_HIQ2Top005_Centrality1030_v, HLT_HIQ2Bottom005_Centrality1030_v, etc)
# HeavyIonUCCDQMSequence * ## OBSOLETE !!!! (looks for HLT_HIUCC100_v and HLT_HIUCC020_v)
# hotlineDQMSequence * ## ORPHAN !!!!
* egammaMonitorHLT
* exoticaMonitorHLT
* susyMonitorHLT
* b2gMonitorHLT
* higgsMonitorHLT
* smpMonitorHLT
* topMonitorHLT
* btagMonitorHLT
* bphMonitorHLT
* hltObjectsMonitor # as online DQM, requested/suggested by TSG coordinators
* jetmetMonitorHLT
)

my naive intuition would be that in case you want to run the corrector for the worfklows starting from RAW, you can modify the definition of offlineHLTSourceOnAOD such that instead of running the sequence jetmetMonitorHLT runs a different sequence (same as the regular one, but without the crashing producer, or a modified producer that could work on AOD).
Please let me know if that makes sense to you.

[1]

$ runTheMatrix.py -nel 136.7801,136.7803

136.7801 RunHLTPhy2017B_AOD+DQMHLTonAOD_2017+HARVESTDQMHLTonAOD_2017 [1]: input from: /HLTPhysics/Run2017B-PromptReco-v1/AOD with run [] 
                                           [2]: cmsDriver.py step2  -s DQM:offlineHLTSourceOnAOD --conditions auto:run2_data --data  --eventcontent DQM --datatier DQMIO --era Run2_2017 --fileout DQMHLTonAOD.root --procModifiers tau_readOldDiscriminatorFormat -n 100 
                                           [3]: cmsDriver.py step3  -s HARVESTING:hltOfflineDQMClient --conditions auto:run2_data --data  --filetype DQM --scenario pp --era Run2_2017 --filein file:DQMHLTonAOD.root -n 100 

136.7803 RunHLTPhy2017B_RAWAOD+DQMHLTonRAWAOD_2017+HARVESTDQMHLTonAOD_2017 [1]: input from: /HLTPhysics/Run2017B-PromptReco-v1/AOD with run [] 
                                           [2]: cmsDriver.py step2  --process reHLT -s HLT:@relval2017,DQM:offlineHLTSourceOnAOD --conditions auto:run2_data --data  --eventcontent DQM --datatier DQMIO --era Run2_2017 --secondfilein filelist:step1_dasparentquery.log --customise_commands "process.HLTAnalyzerEndpath.remove(process.hltL1TGlobalSummary)" --fileout DQMHLTonAOD.root --procModifiers tau_readOldDiscriminatorFormat -n 100 
                                           [3]: cmsDriver.py step3  -s HARVESTING:hltOfflineDQMClient --conditions auto:run2_data --data  --filetype DQM --scenario pp --era Run2_2017 --filein file:DQMHLTonAOD.root -n 100 

2 workflows with 3 steps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi! and thanks for the reply!!
To me (I mean how I understand this fail) the problem is that the ak4PFJetsPuppi collection is not there. The exception in the log file is the following:

----- Begin Fatal Exception 15-Mar-2024 11:53:55 CET-----------------------
An exception of category 'ProductNotFound' occurred while
[0] Processing Event run: 297227 lumi: 1 event: 53500 stream: 0
[1] Running path 'dqmoffline_step'
[2] Calling method for module CorrectedPFJetProducer/'ak4PFJetsPuppiCorrected'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: std::vectorreco::PFJet
Looking for module label: ak4PFJetsPuppi
Looking for productInstanceName:

Additional Info:
[a] If you wish to continue processing events after a ProductNotFound exception,
add "TryToContinue = cms.untracked.vstring('ProductNotFound')" to the "options" PSet in the configuration.

----- End Fatal Exception -------------------------------------------------

Because of the producer. But if for example I use another (not puppi) Jet collection this would not complain. So in order to be sure that in case that our collection is missing, it would not give a fatal exception we should not use the producer and use another way to apply corrections or modify it. But we (JME) want to use Puppi for offline monitoring so, I want to ask because I do not know are they not included in AOD files or this fail is because this is an older workflow and they (Puppi Jets) were not included then?
Thanks, Eirini.

Copy link
Contributor

Choose a reason for hiding this comment

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

I want to ask because I do not know are they not included in AOD files or this fail is because this is an older workflow and they (Puppi Jets) were not included then?

I think this question should be answered by @cms-sw/jetmet-pog-l2 . I see that the failing workflows are from Run2 (2017). If back then puppi jets were not produced it would be understood why the collection is not available in old AOD data . Let me ask again if you really want this to run this on workflows starting from AOD. If yes, I guess you could use an era modifier to remove the producer in the case of 2017 (or other years), otherwise the suggestion at #44411 (comment) still holds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi and thanks for your reply. In my opinion, I think we should keep the sequence there. By applying the offline corrections to be calculated on the fly (after the latest push)

  • runTheMatrix.py -l limited -i all --ibeos and the two that had the fatal exception
  • runTheMatrix.py -l 136.7803 --ibeos
  • runTheMatrix.py -l 136.7801 --ibeos are passing. So, for now I think it is ok.

For the second question I asked, I was trying to understand why the offline Puppi collections were missing and if this is because of the file type AOD.

Thanks a lot for your commends and time,
Eirini.

Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, I think we should keep the sequence there.

removing the sequence was never an option. The suggestion was to modify it such that it can run also on old 2017 AOD data.

Copy link
Contributor

@theochatzis theochatzis Mar 19, 2024

Choose a reason for hiding this comment

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

Hello @mmusich , thanks for all the comments.
I think there is a difference in what each calls sequence above, and just to make sure we have converged on this point I am summarizing the situation :

What Rena was referring above by "removing the sequence" is "removing the corrections sequence" of ak4PFPuppiL1FastL2L3CorrectorChain from here.
So in this sense after her latest push this is not needed to run. The reason is because the "correctors" part will always work even if the ak4PFJetsPuppi does not exist. It only needs the AK4PFPuppi tag in the GT (but that should anyway exist in any GT running in 14_X otherwise all correctors that use it in CMSSW would crash).

Now then in the latest push that she did, the need to have ak4PFJetsPuppi is treated by not producing the corrected jets though the "CorrectedJetsProducer" which would need the collection to work (so in practice modified this sequence for DQM to work in 2017 - Marco's suggestion). The correction is applied then inside the .cc part of the DQM code. If the collection does not exist in this code, like in 2017 AOD files, DQM will just return with no crash.

Copy link
Contributor

@mmusich mmusich left a comment

Choose a reason for hiding this comment

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

few other ones, also please try to stick to a single convention for naming the member variables (e.g. use consistently the trailing underscore "_" for all of them).


muon_1 = v1;
muon_2 = v2;
bool muon_pass = muon_1.Pt() > muon_pt && muon_2.Pt() > muon_pt && abs(muon_1.Eta()) < muon_eta &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bool muon_pass = muon_1.Pt() > muon_pt && muon_2.Pt() > muon_pt && abs(muon_1.Eta()) < muon_eta &&
bool muon_pass = muon_1.Pt() > muon_pt && muon_2.Pt() > muon_pt && std::abs(muon_1.Eta()) < muon_eta &&

muon_1 = v1;
muon_2 = v2;
bool muon_pass = muon_1.Pt() > muon_pt && muon_2.Pt() > muon_pt && abs(muon_1.Eta()) < muon_eta &&
abs(muon_2.Eta()) < muon_eta;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
abs(muon_2.Eta()) < muon_eta;
std::abs(muon_2.Eta()) < muon_eta;

ObjME a_ME_HE_I_mediumRefPt[6];
ObjME a_ME_HE_O_mediumRefPt[6];
ObjME a_ME_HF_mediumRefPt[6];
;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
;

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a9e10a/38155/summary.html
COMMIT: 7aaebe9
CMSSW: CMSSW_14_1_X_2024-03-14-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44411/38155/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

  • 136.7801136.7801_RunHLTPhy2017B_AOD/step2_RunHLTPhy2017B_AOD.log
  • 136.7803136.7803_RunHLTPhy2017B_RAWAOD/step2_RunHLTPhy2017B_RAWAOD.log

Comparison Summary

Summary:

  • You potentially removed 14 lines from the logs
  • Reco comparison results: 2926 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3297383
  • DQMHistoTests: Total failures: 11249
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3286114
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 22855.365 KiB( 47 files compared)
  • DQMHistoSizes: changed ( 1000.0,... ): 2539.485 KiB HLT/JME
  • Checked 202 log files, 165 edm output root files, 48 DQM output files
  • TriggerResults: found differences in 1 / 46 workflows

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44411/39508

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

Pull request #44411 was updated. @cmsbuild, @nothingface0, @syuvivida, @rvenditti, @antoniovagnerini, @tjavaid can you please check and sign again.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44411/39712

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

Pull request #44411 was updated. @nothingface0, @rvenditti, @cmsbuild, @antoniovagnerini, @tjavaid, @syuvivida can you please check and sign again.

@mmusich
Copy link
Contributor

mmusich commented Mar 27, 2024

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a9e10a/38473/summary.html
COMMIT: 6b0ed49
CMSSW: CMSSW_14_1_X_2024-03-27-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44411/38473/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 16 lines to the logs
  • Reco comparison results: 38 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3297857
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3297837
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 22855.365 KiB( 47 files compared)
  • DQMHistoSizes: changed ( 1000.0,... ): 2539.485 KiB HLT/JME
  • Checked 202 log files, 165 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@mmusich
Copy link
Contributor

mmusich commented Apr 2, 2024

@cms-sw/dqm-l2 can you please review this? It would be good to have this integrated once we deliver the next HLT menu 2024 V1.1

@tjavaid
Copy link

tjavaid commented Apr 3, 2024

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2024

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @antoniovilela, @sextonkennedy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@antoniovilela
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit f34cb55 into cms-sw:master Apr 3, 2024
11 checks passed
@mmusich
Copy link
Contributor

mmusich commented Apr 4, 2024

@esiam will you prepare a 14.0.x backport?

@esiam
Copy link
Contributor Author

esiam commented Apr 4, 2024

Hi Marco, yes I will. Thank you!

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.

6 participants