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

addition of new MET filter BadPFMuonDzFilter_cfi.py in CMSSW_10_6_X : i.e. backport from 11_1_X of PR 30015 and 30974 #31087

Merged
merged 4 commits into from Sep 14, 2020

Conversation

amkalsi
Copy link
Contributor

@amkalsi amkalsi commented Aug 7, 2020

This PR is created for backport from CMSSW_11_1_X (PR 30015 and 30974) which is addition of new MET filter : BadPFMuonDzFilter_cfi.py and corresponding change in extraflags_cff file for nanoAOD setup
It includes all the changes in format of other files of BadParticleFilter.cc module which were done for PR 30015

This PR includes also the backport of #31432 ( addition of BadPFMuonDzFilter plot in nanoDQM_cfi)

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 7, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 7, 2020

-code-checks

ERROR: Build errors found during clang-tidy run.

gmake: *** [config/SCRAM/GMake/Makefile.coderules:128: code-checks] Error 2
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

@slava77
Copy link
Contributor

slava77 commented Aug 7, 2020

@amkalsi
please click "Edit" and select base CMSSW_10_6_X

@amkalsi amkalsi changed the base branch from master to CMSSW_10_6_X August 7, 2020 07:21
@amkalsi
Copy link
Contributor Author

amkalsi commented Aug 7, 2020

Thanks @slava77 . it is done

@mariadalfonso
Copy link
Contributor

@amkalsi
do we need here also the back port of the #30974 (nanoConfig) otherwise will crash.

@cmsbuild cmsbuild removed this from the CMSSW_11_2_X milestone Aug 7, 2020
@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2020

+1
Tested at: 2bc1b1c
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2d0a0d/9221/summary.html
CMSSW: CMSSW_10_6_X_2020-09-09-1100
SCRAM_ARCH: slc7_amd64_gcc700

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2020

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2020

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2d0a0d/9221/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 142 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 3214631
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3214293
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.276 KiB( 33 files compared)
  • DQMHistoSizes: changed ( 10024.0,... ): 0.004 KiB MessageLogger/Errors
  • DQMHistoSizes: changed ( 10024.0,... ): 0.004 KiB MessageLogger/Warnings
  • DQMHistoSizes: changed ( 1325.7 ): 0.116 KiB Physics/NanoAODDQM
  • Checked 140 log files, 14 edm output root files, 34 DQM output files

@slava77
Copy link
Contributor

slava77 commented Sep 10, 2020

@amkalsi
Related to the change in PhysicsTools/NanoAOD/python/nanoDQM_cfi.py, which is not yet in the master.
I intend/prefer to sign only after the master version includes the relevant updates so that this is actually a backport and not a new development.

please clarify on the status of updating the master version of PhysicsTools/NanoAOD/python/nanoDQM_cfi.py to include BadPFMuonDzFilter.

Thank you.

@amkalsi
Copy link
Contributor Author

amkalsi commented Sep 11, 2020

@amkalsi
Related to the change in PhysicsTools/NanoAOD/python/nanoDQM_cfi.py, which is not yet in the master.
I intend/prefer to sign only after the master version includes the relevant updates so that this is actually a backport and not a new development.

please clarify on the status of updating the master version of PhysicsTools/NanoAOD/python/nanoDQM_cfi.py to include BadPFMuonDzFilter.

Thank you.

Hi @slava77
Please have a look at the pull request made to CMSSW_11_2_X to include BadPFMuonDzFilter in PhysicsTools/NanoAOD/python/nanoDQM_cfi.py
#31432

@slava77
Copy link
Contributor

slava77 commented Sep 11, 2020

+1

for #31087 2bc1b1c

  • code changes are in line with the PR description and the follow up review
    • the backported code is enabled by default for the new filter path Flag_BadPFMuonDzFilter, which I think can be acceptable within the "no-change" requirements. For the combined Flag_metFilters, which is already running in production/old workflows the new BadPFMuonDzFilter is added only for run2_miniAOD_UL, in agreement with the no-change rules.
  • jenkins tests pass and comparisons with the baseline show differences in the count (and order) of entries in the TriggerResults

@slava77
Copy link
Contributor

slava77 commented Sep 11, 2020

@silviodonato @qliphy @cms-sw/xpog-l2
this PR will apparently conflict with #31413 . I propose to integrate this one first (fifo).

@santocch
Copy link

+1

@slava77
Copy link
Contributor

slava77 commented Sep 14, 2020

@cms-sw/xpog-l2

your signature is needed for this PR.
Please take a look.
Thank you.

@mariadalfonso
Copy link
Contributor

+xpog
new MET filters added in the mini and nano available only with the UL_remini

@amkalsi
in the description can you also mention the backport of PR #31432

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_10_6_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_11_2_X is complete. 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)

@mariadalfonso
Copy link
Contributor

@amkalsi @lathomas
As this PR only add the filter if re-mini are made, can you investigate the possibility to re-run on top of the existing old mini ? so in case people make their private nano or a central renano only happens shortly.

Please add this to master first.

@silviodonato
Copy link
Contributor

+1

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

9 participants