Updated PR for CorrWithOverlapRemovalCondition.cc

[namppl]

PR description:

Update of #36103. Rebased to the latest commit.

PR validation:

Unit tests done

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36758/27881

• This PR adds an extra 24KB to repository

• There are other open Pull requests which might conflict with changes you have proposed:

  • File L1Trigger/L1TGlobal/src/CorrWithOverlapRemovalCondition.cc modified in PR(s): fixed CorrWithOverlapRemovalCondition.cc #36103

[cmsbuild]

A new Pull Request was created by @namppl (Kyungwook Nam) for master.

It involves the following packages:

• L1Trigger/L1TGlobal (l1)

@epalencia, @cmsbuild, @cecilecaillol, @rekovic can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @missirol 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

[cecilecaillol]

please test

[rekovic]

This PR is the change of logic from the one meant by OverlapRemoval. In the logic of this PR there is no OverlapRemoval but rather a condition of three objects with correlation conditions imposed on pairs of two. No L1T object is removed !!

The logic of this PR is what I implemented in the new class CorrThreeBodyByTwoBodyCondition #36441 which was resently reverted and which I am now fixing. Development of the logic of this PR needs to continue on that class and not touch the CorrWithOverlapRemovalCondition which does something very different, removes overlapping objects at L1T, many of them, to reduce the fake rate.

Both emulators need to be made available in the GlobalTrigger emulation and part of the CMSSW release for physics (May), This PR destroys and hijacks the ongoing developments of OverlapRemoval which tries to improve the GlobalTrigger by ridding the overlaps. Since the proponents of the new condition did not bother developing the emulator for their proposals, I am making one for them!! The hope is to have the GlobalTrigger emulator fully supporting this logic in the ThreeByTwoBodyCorrelation and the one of true OverlapRemoval next week.

[cmsbuild]

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0a57b0/21844/summary.html
COMMIT: 48ffc5f
CMSSW: CMSSW_12_3_X_2022-01-20-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36758/21844/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

• 4.764.76_ZMuSkim2012D+ZMuSkim2012D+HLTDSKIM2+RECODR1reHLT2+HARVESTDR1reHLT/step2_ZMuSkim2012D+ZMuSkim2012D+HLTDSKIM2+RECODR1reHLT2+HARVESTDR1reHLT.log

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 4 differences found in the comparisons
• DQMHistoTests: Total files compared: 43
• DQMHistoTests: Total histograms compared: 3464860
• DQMHistoTests: Total failures: 5
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3464833
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
• Checked 181 log files, 42 edm output root files, 43 DQM output files
• TriggerResults: no differences found

[cecilecaillol]

please test

[cecilecaillol]

+l1

[cmsbuild]

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

[cmsbuild]

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0a57b0/21897/summary.html
COMMIT: 48ffc5f
CMSSW: CMSSW_12_3_X_2022-01-21-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36758/21897/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

• 4.764.76_ZMuSkim2012D+ZMuSkim2012D+HLTDSKIM2+RECODR1reHLT2+HARVESTDR1reHLT/step2_ZMuSkim2012D+ZMuSkim2012D+HLTDSKIM2+RECODR1reHLT2+HARVESTDR1reHLT.log

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 4 differences found in the comparisons
• DQMHistoTests: Total files compared: 43
• DQMHistoTests: Total histograms compared: 3464860
• DQMHistoTests: Total failures: 5
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3464833
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
• Checked 181 log files, 42 edm output root files, 43 DQM output files
• TriggerResults: no differences found

[perrotta]

@cecilecaillol I don't think that @rekovic agrees with the content of this PR, see #36758 (comment)
Could you please agree among yourself a common @cms-sw/l1-l2 decision for this PR

I'll keep this PR on hold while waiting for such additional information

[perrotta]

hold

[cmsbuild]

Pull request has been put on hold by @perrotta
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

[cecilecaillol]

@perrotta Merging this PR is supported by L1 DPG conveners and L1 project managers

[lathomas]

@perrotta I confirm that the strategy of this PR is blessed by L1 DPG. We would need it to be merged as soon as possible for seed developments that need to make it in 12_3. Thanks !

[perrotta]

unhold

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). 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)

[perrotta]

+1

• Comment Updated PR for CorrWithOverlapRemovalCondition.cc #36758 (comment) from @rekovic is superseded by Updated PR for CorrWithOverlapRemovalCondition.cc #36758 (comment) from @cecilecaillol and Updated PR for CorrWithOverlapRemovalCondition.cc #36758 (comment) from @lathomas
• @cecilecaillol @lathomas thank you for the confirmation; next time please answer directly to the comment that suggests to reject this PR, and the review can proceed smoothly

[perrotta]

merge