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

Update tau anti electron discriminators in RECO and MiniAOD (10_6_X) #31065

Conversation

mbluj
Copy link
Contributor

@mbluj mbluj commented Aug 5, 2020

PR description:

This PR updates the version of the tau anti-electron discriminator MVA6 in miniAOD to the latest 2018 version with run2_miniAOD_UL modifier used - it is intended to be a part of UL re-miniAOD.

The NanoAOD configs are also updated to accommodate the changes in miniAOD. In addition the anti-electron-in-deadECal tauID is added to NanoAOD.

Note: updates for nanoAOD are still governed by the run2_miniAOD_devel modifier and require a followup PR (probably central one by the NanoAOD team).

Warning: RECO/AOD content will be also modified when RECO sequence run (by mistake) with the run2_miniAOD_UL modifier used.

PR validation:

Verified with custom configurations to produce miniAOD and nanoAOD (with cmsDriver) that changes are as expected for run2_miniAOD_UL or `run2_miniAOD_devel modifier used and that nothing changes when none of the modifiers is used.

if this PR is a backport please specify the original PR and why you need to backport that PR:

Can be seen as a backport of #27465 for coherency of UL re-miniAOD with 10_6_X and 11_2_X as discussed in #27465 (comment)
Adding of the anti-electron-in-deadECal tauID to NanoAOD is backport of #31077.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 5, 2020

A new Pull Request was created by @mbluj for CMSSW_10_6_X.

It involves the following packages:

PhysicsTools/PatAlgos
RecoTauTag/Configuration

@perrotta, @jpata, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @gouskos, @hatakeyamak, @emilbols, @peruzzim, @seemasharmafnal, @mmarionncern, @ahinzmann, @smoortga, @jdolen, @ferencek, @jdamgov, @nhanvtran, @gkasieczka, @schoef, @andrzejnovak, @clelange, @riga, @JyothsnaKomaragiri, @mverzett, @gpetruc, @mariadalfonso this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@mbluj
Copy link
Contributor Author

mbluj commented Aug 6, 2020

As discussed with nanoAOD team here cms-nanoAOD#535 this PR will be extended to adjust also NanoAOD sequences to update of anti-e discriminators at miniAOD (as in backported #27465).
In addition anti-e in deadEcal will be added to nanoAOD (can be treated as backport of #31077).

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2020

Pull request #31065 was updated. @perrotta, @gouskos, @cmsbuild, @fgolf, @slava77, @jpata, @mariadalfonso, @santocch, @peruzzim can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Aug 6, 2020

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2020

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2020

+1
Tested at: 1138f5f
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e73ac0/8629/summary.html
CMSSW: CMSSW_10_6_X_2020-08-06-1100
SCRAM_ARCH: slc7_amd64_gcc700

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2020

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2020

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 3214618
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3214282
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.242 KiB( 33 files compared)
  • DQMHistoSizes: changed ( 1325.7 ): 0.242 KiB Physics/NanoAODDQM
  • Checked 140 log files, 14 edm output root files, 34 DQM output files

@slava77
Copy link
Contributor

slava77 commented Aug 11, 2020

there is a merge conflict here now.

Please rebase and possibly switch to run2_miniAOD_UL in the parts that affect miniAOD (nanoAOD could stay with _devel, I suppose) so that there is no need for a follow up PR to enable the miniAOD part.
The coupling to deadECAL can apparently be avoided in the reco/miniAOD files; although it's less clear about the nanoAOD part.
Perhaps it's better to wait until at least the reco/mini part of deadEcal is merged.

@mariadalfonso
Copy link
Contributor

@mbluj
ok thanks for the investigations.

The problem came when we had the (~run2_miniAOD_80XLegacy) ?
Seems is there in the release since 'Oct 2019', we should fix master 11_2.
Do you see the same problem also in the current 10_6 ?

@cmsbuild
Copy link
Contributor

+1
Tested at: c554314
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e73ac0/8962/summary.html
CMSSW: CMSSW_10_6_X_2020-08-27-1100
SCRAM_ARCH: slc7_amd64_gcc700

@cmsbuild
Copy link
Contributor

Comparison job queued.

@mbluj
Copy link
Contributor Author

mbluj commented Aug 27, 2020

@mbluj
ok thanks for the investigations.

The problem came when we had the (~run2_miniAOD_80XLegacy) ?

Yes, the issue was inducted when (~run2_miniAOD_80XLegacy) replaced list of modifiers.

Seems is there in the release since 'Oct 2019', we should fix master 11_2.

Yes, it is in CMSSW for some time already. It was not found as for eras used for testing the logic is correct and as releases newer than 106X are not used to produce nanoAOD. I will prepare a fix to master (hopefully tomorrow).

Do you see the same problem also in the current 10_6 ?

I think it is OK as the issue was introduced to some 11XY release.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 89 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 3214618
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3214281
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.242 KiB( 33 files compared)
  • DQMHistoSizes: changed ( 1325.7 ): 0.242 KiB Physics/NanoAODDQM
  • Checked 140 log files, 14 edm output root files, 34 DQM output files

@slava77
Copy link
Contributor

slava77 commented Aug 28, 2020

+1

for #31065 c554314

@santocch
Copy link

+1

@slava77
Copy link
Contributor

slava77 commented Aug 31, 2020

@cms-sw/xpog-l2
please check if the last updates are OK for you.
Thank you.

@mbluj
Copy link
Contributor Author

mbluj commented Aug 31, 2020

Requested plots comparing anti-e variables between this PR and master (actually master + #31302 in case of 2015 training) both using UL re-miniAOD with 10_6_X (RelVal_ZTT); red histogram for nanoAOD with 10_6_X and run2_miniAOD_devel, blue histogram with error bars for nanoAOD with 11_2_X and without any era modifier:

  • anti-e MVA tauID 2018 training (Tau_rawAntiEle2018):
    Tau_rawAntiEle2018_Devel-106XvsNoEra-master
  • anti-e MVA tauID 2018 training (Tau_rawAntiEle):
    Tau_rawAntiEle_Devel-106XvsNoEra-master

@mariadalfonso
Copy link
Contributor

@mbluj
Thanks for these plots.

@mariadalfonso
Copy link
Contributor

+xpog
anti-e-in-deadECal against dead ECAL added for all the nano with any mini-eras
anti-electron ID updated for the run2_miniAOD_devel (unchanged in previous eras)

@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)

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 91baf1b into cms-sw:CMSSW_10_6_X Sep 1, 2020
@mbluj mbluj deleted the CMSSW_10_6_X_tau-pog_updateAntiEleDiscr branch October 10, 2023 10:03
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

8 participants