Fix deg to rad conversion in L1TkMuonProducer ("ManTra" version) for HLT TDR [11_2_X]

[dildick]

PR description:

An inefficiency was reported for the L1TkMu object in recently produced samples (including /DYToLL_M-50_TuneCP5_14TeV-pythia8/Phase2HLTTDRSummer20ReRECOMiniAOD-NoPU_pilot_111X_mcRun4_realistic_T15_v1-v1/FEVT) for the HLT TDR. Per @l-cadamuro 's suggestion I changed the deg_to_rad function back to the definition in https://github.com/cms-l1t-offline/cmssw/blob/l1t-phase2-v2.37.2/L1Trigger/L1TTrackMatch/interface/L1TkMuMantra.h#L85.

PR validation:

Tested the TP, DynamicWindows and ManTra versions on 2900 events in /store/mc/Phase2HLTTDRSummer20ReRECOMiniAOD/DYToLL_M-50_TuneCP5_14TeV-pythia8/FEVT/NoPU_pilot_111X_mcRun4_realistic_T15_v1-v1/100000/FE80A42E-0B59-FD45-A1E5-56A34EFA3DB9.root. Tested the ManTra version again after the fix on the same sample. Efficiency plots can be found here: https://its.cern.ch/jira/browse/CMSLITDPG-908. This largely fixes the issue, though a substantial drop can be seen near |eta|~1.6.

ManTra before fix:

• pt https://its.cern.ch/jira/secure/attachment/249235/Screen%20Shot%202020-09-02%20at%203.44.31%20PM.png
• |eta|: https://its.cern.ch/jira/secure/attachment/249236/Screen%20Shot%202020-09-02%20at%203.46.16%20PM.png
• phi: https://its.cern.ch/jira/secure/attachment/249234/Screen%20Shot%202020-09-02%20at%203.43.36%20PM.png

ManTra after fix:

• pt: https://its.cern.ch/jira/secure/attachment/249239/Screen%20Shot%202020-09-02%20at%209.36.18%20PM.png
• |eta|: https://its.cern.ch/jira/secure/attachment/249240/Screen%20Shot%202020-09-02%20at%209.36.56%20PM.png
• phi: https://its.cern.ch/jira/secure/attachment/249241/Screen%20Shot%202020-09-02%20at%209.37.29%20PM.png

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

May need to be backported to 11_1_X

FYI @l-cadamuro @rekovic @BenjaminRS

[cmsbuild]

The code-checks are being triggered in jenkins.

[dildick]

@cmsbuild urgent

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31341/18113

• This PR adds an extra 12KB to repository

[cmsbuild]

A new Pull Request was created by @dildick (Sven Dildick) for master.

It involves the following packages:

L1Trigger/L1TTrackMatch

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

[dpiparo]

sorry, I commented on the backport: the very same routine seems to be available here: https://cmssdt.cern.ch/lxr/source/DataFormats/Math/interface/angle_units.h

[dildick]

@l-cadamuro, can you comment on @dpiparo's question?

[l-cadamuro]

Hi @dpiparo, no specific reason, it's just that when porting that code to CMSSW it was not modified to use the angle_units class (plus I was unaware of the existence of that routine).
No preference for me on whether keeping the code as is now or update it to use that method, they implement exactly the same operation

[silviodonato]

I would go ahead with the current fix and I would ask @dildick / @l-cadamuro / @rekovic to make a general cleaning of deg_to_rad and of rad_to_deg in L1T code (no backport is needed)

https://cmssdt.cern.ch/lxr/search?%21v=CMSSW_11_2_X_2020-09-02-2300&_filestring=&_string=deg_to_rad
https://cmssdt.cern.ch/lxr/search?%21v=CMSSW_11_2_X_2020-09-02-2300&_filestring=&_string=rad_to_deg

[silviodonato]

please test

[cmsbuild]

The tests are being triggered in jenkins.

• CMSSW_11_2_X_2020-09-03-1100/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/9101/console Started: 2020/09/03 17:38

[cmsbuild]

+1
Tested at: a9ab4f4
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ff6182/9101/summary.html
CMSSW: CMSSW_11_2_X_2020-09-03-1100
SCRAM_ARCH: slc7_amd64_gcc820

[cmsbuild]

Comparison job queued.

[davidlange6]

As L1 tends not to make changes like this quickly, perhaps one can agree on a timescale for this happening? On Sep 3, 2020, at 5:37 PM, Silvio Donato <notifications@github.com<mailto:notifications@github.com>> wrote: I would go ahead with the current fix and I would ask @dildick<https://github.com/dildick> / @l-cadamuro<https://github.com/l-cadamuro> / @rekovic<https://github.com/rekovic> to make a general cleaning of deg_to_rad and of rad_to_deg in L1T code (no backport is needed) https://cmssdt.cern.ch/lxr/search?%21v=CMSSW_11_2_X_2020-09-02-2300&_filestring=&_string=deg_to_rad https://cmssdt.cern.ch/lxr/search?%21v=CMSSW_11_2_X_2020-09-02-2300&_filestring=&_string=rad_to_deg — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#31341 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABGPFQYAF2PCYZLWYNAVYULSD6Z4NANCNFSM4QUF7LUQ>.

[cmsbuild]

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

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 35
• DQMHistoTests: Total histograms compared: 2609667
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2609644
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
• Checked 149 log files, 22 edm output root files, 35 DQM output files

[dildick]

@silviodonato For changes to EMTF, @jiafulow is probably the right person to do this. @jiafulow A proposal: https://gist.github.com/dildick/dcd2240c164f3c9b695b58ad0f6d06b5.

[cmsbuild]

+1
Tested at: ab1423a
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ff6182/9127/summary.html
CMSSW: CMSSW_11_2_X_2020-09-04-1100
SCRAM_ARCH: slc7_amd64_gcc820

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 35
• DQMHistoTests: Total histograms compared: 2609667
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2609644
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)
• Checked 149 log files, 22 edm output root files, 35 DQM output files

[rekovic]

+1

[rekovic]

@dildick @l-cadamuro Thanks.
Once merged, please provide a backport to 11_1_X.

[dildick]

Thanks Vladimir. Please see #31342 for the 11_1_X version.

[kpedro88]

+upgrade

[cmsbuild]

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

[silviodonato]

+1

[silviodonato]

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

Comparison Summary:

* No significant changes to the logs found

* Reco comparison results: 0 differences found in the comparisons

* DQMHistoTests: Total files compared: 35

* DQMHistoTests: Total histograms compared: 2609667

* DQMHistoTests: Total failures: 1

* DQMHistoTests: Total nulls: 0

* DQMHistoTests: Total successes: 2609644

* DQMHistoTests: Total skipped: 22

* DQMHistoTests: Total Missing objects: 0

* DQMHistoSizes: Histogram memory added: 0.0 KiB( 34 files compared)

* Checked 149 log files, 22 edm output root files, 35 DQM output files

@rekovic @dildick this remind us that we definitely need a DQM for the phase-2 L1T!