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

[CSC DT RPC] Bug fix to tag-and-probe in DQM #38028

Merged
merged 2 commits into from May 26, 2022

Conversation

llunerti
Copy link

@llunerti llunerti commented May 20, 2022

PR description:

This PR fixes an issue in the tag-and-probe worflow appeared since 12_4_0_pre3. As you can see e.g. here [1] all CSC plots are empty. All DQM plots are empty also for DT and RPC. The reason is that the tag muon trigger matching always fails.

The trigger matching was originally performed by checking the second last HLT filter (the last filter was always hltBoolEnd). Since 12_4_0_pre3 it seems that additional filters appeared after hltBoolEnd and the HLT filter to check is not the second last one anymore. This problem has been fixed by simply selecting the HLT filter before hltBoolEnd and not assuming that it was always the second last.

[1] https://tinyurl.com/y6dn23ye

PR validation:

This fix has been validated by running the following test in 12_4_0_pre3 release:

runTheMatrix.py -l 11650.0 -i all --ibeos (to check that output plots are not empty) and runTheMatrix.py -l limited -i all --ibeos showed no failed tests

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38028/30102

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • DQMOffline/MuonDPG (dqm)

@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @micsucmed, @rvenditti can you please review it and eventually sign? Thanks.
@sscruz, @barvic, @bellan, @abbiendi, @Fedespring, @calderona, @HuguesBrun, @jhgoh, @ptcox, @trocino, @cericeci, @rociovilar this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@missirol
Copy link
Contributor

Since 12_4_0_pre3 it seems that additional filters appeared after hltBoolEnd [..].

Mh.. I don't think this should be happening.

@llunerti , what are the filters you find after hltBoolEnd, and which HLT paths are you considering in this DQM workflow? Could you please share a recipe to reproduce the problem?

@emanueleusai
Copy link
Member

please test

@emanueleusai
Copy link
Member

@llunerti can you please mention the subdetectors in the name of the PR? e.g. "[CSC DT RPC] Bug fix to tag-and-probe in DQM"

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c5bddc/24891/summary.html
COMMIT: b977224
CMSSW: CMSSW_12_5_X_2022-05-20-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38028/24891/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: 8 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3672423
  • DQMHistoTests: Total failures: 13
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3672387
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 49 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 208 log files, 45 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@@ -187,7 +188,9 @@ bool BaseTnPEfficiencyTask::hasTrigger(std::vector<int>& trigIndices,
float dR2match = 999.;
for (int trigIdx : trigIndices) {
const std::vector<std::string> trigModuleLabels = m_hltConfig.moduleLabels(trigIdx);
Copy link
Author

Choose a reason for hiding this comment

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

@missirol I'm not sure how to give you a recipe to reproduce the problem, but if here I simply print out the module labels inside trigModuleLabels, these are the last filters in one of the event that I see in 12_4_0_pre3:

--> moduleLabels[296]: hltHpsSelectedPFTau27TightChargedIsolationAgainstMuonL1HLTMatched
--> moduleLabels[297]: hltHpsOverlapFilterIsoMu20TightChargedIsoPFTau27L1Seeded
--> moduleLabels[298]: hltBoolEnd
--> moduleLabels[299]: hltEcalRecHit@cpu
--> moduleLabels[300]: hltEcalUncalibRecHit@cpu
--> moduleLabels[301]: hltHbhereco@cpu
--> moduleLabels[302]: hltPixelTracksSoA@cpu
--> moduleLabels[303]: hltPixelVerticesSoA@cpu

while in 12_2_0_pre2 (that I used to test the first PR) I find e.g. :

--> moduleLabels[442]: hltPFTau27TrackLooseChargedIsoAgainstMuon
--> moduleLabels[443]: hltL1JetsHLTPFTauTrackLooseChargedIsolationAgainstMuonMatch
--> moduleLabels[444]: hltSelectedPFTau27LooseChargedIsolationAgainstMuonL1HLTMatched
--> moduleLabels[445]: hltOverlapFilterIsoMu20LooseChargedIsoPFTau27L1Seeded
--> moduleLabels[446]: hltBoolEnd

Copy link
Contributor

@missirol missirol May 21, 2022

Choose a reason for hiding this comment

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

Since 12_4_0_pre3 it seems that additional filters appeared after hltBoolEnd [..].

@llunerti , okay, thanks. Now I understand. Those are not "additional filters" (or, EDFilters), they are EDProducers used in that trigger path. The reason they now appear at the end of this list goes back to a technical change in the HLT menu (those modules are now in Tasks and not Sequences, see #37783 if interested in the details). I don't know this DQM plugin, but here it looks like you are trying to select the EDFilter of a given HLT Path before the final hltBoolEnd, and you could do that as follows (untested code):

auto const& trigModuleLabels = m_hltConfig.moduleLabels(trigIdx);
if (trigModuleLabels.empty()) { continue; }

int trigModuleIndex = -1;
for(uint midx = trigModuleLabels.size()-1; midx == 0; --midx){
  auto const& moduleLabel = trigModuleLabels[midx];
  if (moduleLabel != "hltBoolEnd" and m_hltConfig.moduleEDMType(moduleLabel) == "EDFilter") {
    trigModuleIndex = midx;
    break;
  }
}
if (trigModuleIndex < 0) { continue; }

Copy link
Author

Choose a reason for hiding this comment

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

@missirol thanks for the clarification! Shouldn't also work my solution? Or am I missing something?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I missed this message. My understanding is that your solution will work just as well for the cases you are considering (because the HLT paths monitored here will/should always have at least one filter, and the module before hltBooEnd would always be a EDFilter). At the same time, the implementation is a bit fragile, as these conditions are not true in general (for example, in trigger paths for Scouting, the module before hltBoolEnd is not a EDFilter). My opinion is that, with a slightly more solid implementation, you might save yourself (or others) trouble in the future (this PR itself wouldn't have been necessary if the implementation had been a bit more solid).

@llunerti llunerti changed the title Bug fix to tag-and-probe in DQM [CSC DT RPC] Bug fix to tag-and-probe in DQM May 21, 2022
@emanueleusai
Copy link
Member

+1

@cmsbuild
Copy link
Contributor

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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

const unsigned trigModuleIndex = trigModuleLabels.size() - 2;

const unsigned trigModuleIndex =
std::find(trigModuleLabels.begin(), trigModuleLabels.end(), "hltBoolEnd") - trigModuleLabels.begin() - 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not work if trigModuleLabels is empty.
I imagine that there is something elsewehere that prevents it being empty by construction, is it correct?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, basically trigModuleLabels is not empty if there is at least a path matching IsoMu trigger (this is done here inside dqmBeginRun). If no matching path are found then trigIndices is empty and the loop inside hasTrigger won't run.

@perrotta
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit e48b3c4 into cms-sw:master May 26, 2022
cmsbuild added a commit that referenced this pull request Jun 24, 2022
[DT CSC RPC] Backport of bug fix to tag-and-probe in DQM (#38028)
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

5 participants