Use std:: functions instead of TMath:: functions in RecoEgamma #34325

guitargeek · 2021-07-04T00:07:23Z

PR description:

I think the standard library math functions should be used unless there is a specific reason to use something else.

RecoEgamma already uses the standard library functions for the most part, this PR is replacing the remaining TMath functions.

PR validation:

CMSSW compiles.

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

No backport indended.

cmsbuild · 2021-07-04T00:16:01Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34325/23674

This PR adds an extra 80KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoEgamma/Examples/plugins/GsfElectronDataAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/Examples/plugins/GsfElectronFakeAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/Examples/plugins/GsfElectronMCAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/Examples/plugins/GsfElectronMCFakeAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/PhotonIdentification/src/PhotonIsolationCalculator.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34325/23674/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34325/23674/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

guitargeek · 2021-07-04T17:33:06Z

Any idea why the code checks didn't get triggered again?

smuzaffar · 2021-07-04T17:37:34Z

code-checks

no idea, may be you pushed a commit with timestamp in past?

cmsbuild · 2021-07-04T17:45:28Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34325/23679

This PR adds an extra 8KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoEgamma/Examples/plugins/GsfElectronDataAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/Examples/plugins/GsfElectronFakeAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/Examples/plugins/GsfElectronMCAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/Examples/plugins/GsfElectronMCFakeAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/PhotonIdentification/src/PhotonIsolationCalculator.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666

cmsbuild · 2021-07-04T17:45:53Z

A new Pull Request was created by @guitargeek (Jonas Rembser) for master.

It involves the following packages:

RecoEgamma/EgammaElectronAlgos
RecoEgamma/EgammaPhotonAlgos
RecoEgamma/EgammaTools
RecoEgamma/Examples
RecoEgamma/PhotonIdentification

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@Sam-Harper, @jainshilpi, @lgray, @sobhatta, @afiqaize, @varuns23, @ram1123 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

slava77 · 2021-07-04T18:49:46Z

@cmsbuild please test

cmsbuild · 2021-07-04T21:58:17Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-799f36/16452/summary.html
COMMIT: 6499bbf
CMSSW: CMSSW_12_0_X_2021-07-04-0000/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34325/16452/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS StaticAnalyzer warnings: There are 6 EventSetupRecord::get warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-799f36/16452/llvm-analysis/esrget-sa.txt for details.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2785924
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2785895
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: no differences found

VinInn · 2021-07-05T10:35:59Z

maybe also
Double_t -> double
Bool_t -> bool
?

slava77 · 2021-07-05T13:46:10Z

maybe also
Double_t -> double
Bool_t -> bool
?

@guitargeek
please apply these type name changes as well.
Thank you.

cmsbuild · 2021-07-05T15:24:58Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34325/23697

This PR adds an extra 36KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoEgamma/Examples/plugins/GsfElectronDataAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/Examples/plugins/GsfElectronFakeAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/Examples/plugins/GsfElectronMCAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/Examples/plugins/GsfElectronMCFakeAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/PhotonIdentification/src/PhotonIsolationCalculator.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34325/23697/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34325/23697/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-07-05T15:50:59Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34325/23702

This PR adds an extra 44KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoEgamma/Examples/plugins/GsfElectronDataAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/Examples/plugins/GsfElectronFakeAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/Examples/plugins/GsfElectronMCAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/Examples/plugins/GsfElectronMCFakeAnalyzer.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666
- File RecoEgamma/PhotonIdentification/src/PhotonIsolationCalculator.cc modified in PR(s): Calotower -> HCAL rechit for EGM ID variables #33666

cmsbuild · 2021-07-05T15:51:24Z

Pull request #34325 was updated. @perrotta, @jpata, @cmsbuild, @slava77 can you please check and sign again.

perrotta · 2021-07-06T07:54:33Z

please test

cmsbuild · 2021-07-06T15:58:48Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-799f36/16487/summary.html
COMMIT: 0670d7a
CMSSW: CMSSW_12_0_X_2021-07-05-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34325/16487/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS StaticAnalyzer warnings: There are 6 EventSetupRecord::get warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-799f36/16487/llvm-analysis/esrget-sa.txt for details.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2785924
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2785901
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: no differences found

slava77 · 2021-07-07T15:33:29Z

+reconstruction

for #34325 0670d7a

code changes are technical, they are in line with the PR description and the follow up review
jenkins tests pass and comparisons with the baseline show no differences

cmsbuild · 2021-07-07T15:33:56Z

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)

qliphy · 2021-07-07T15:46:02Z

+1

Repace TMath:: functions with std:: functions in RecoEgamma

6499bbf

cmsbuild added this to the CMSSW_12_0_X milestone Jul 4, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Jul 4, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jul 4, 2021

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jul 4, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 4, 2021

cmsbuild added tests-started and removed tests-pending labels Jul 4, 2021

cmsbuild added tests-approved and removed tests-started labels Jul 4, 2021

cmsbuild added tests-started and removed tests-approved labels Jul 5, 2021

Repace Bool_t with bool and Double_t with double in RecoEgamma

d138052

cmsbuild removed the code-checks-approved label Jul 5, 2021

cmsbuild added the tests-pending label Jul 5, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jul 5, 2021

Apply clang-format to newvalidation.C and eleValidation.C

0670d7a

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jul 5, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 5, 2021

cmsbuild added tests-started and removed tests-pending labels Jul 6, 2021

cmsbuild added tests-approved and removed tests-started labels Jul 6, 2021

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Jul 7, 2021

cmsbuild added orp-approved and removed orp-pending labels Jul 7, 2021

cmsbuild merged commit db22c7c into cms-sw:master Jul 7, 2021

guitargeek deleted the RecoEgamma_TMath branch July 7, 2021 15:48

This was referenced Jul 7, 2021

Remove duplicate service from T25&T26 #34388

Merged

cleanup python2; use rpm macros for python versions cms-sw/cmsdist#7106

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use std:: functions instead of TMath:: functions in RecoEgamma #34325

Use std:: functions instead of TMath:: functions in RecoEgamma #34325

guitargeek commented Jul 4, 2021

cmsbuild commented Jul 4, 2021

guitargeek commented Jul 4, 2021

smuzaffar commented Jul 4, 2021

cmsbuild commented Jul 4, 2021

cmsbuild commented Jul 4, 2021

slava77 commented Jul 4, 2021

cmsbuild commented Jul 4, 2021

VinInn commented Jul 5, 2021

slava77 commented Jul 5, 2021

cmsbuild commented Jul 5, 2021

cmsbuild commented Jul 5, 2021

cmsbuild commented Jul 5, 2021

perrotta commented Jul 6, 2021

cmsbuild commented Jul 6, 2021

slava77 commented Jul 7, 2021

cmsbuild commented Jul 7, 2021

qliphy commented Jul 7, 2021

Use std:: functions instead of TMath:: functions in RecoEgamma #34325

Use std:: functions instead of TMath:: functions in RecoEgamma #34325

Conversation

guitargeek commented Jul 4, 2021

PR description:

PR validation:

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

cmsbuild commented Jul 4, 2021

guitargeek commented Jul 4, 2021

smuzaffar commented Jul 4, 2021

cmsbuild commented Jul 4, 2021

cmsbuild commented Jul 4, 2021

slava77 commented Jul 4, 2021

cmsbuild commented Jul 4, 2021

Comparison Summary

VinInn commented Jul 5, 2021

slava77 commented Jul 5, 2021

cmsbuild commented Jul 5, 2021

cmsbuild commented Jul 5, 2021

cmsbuild commented Jul 5, 2021

perrotta commented Jul 6, 2021

cmsbuild commented Jul 6, 2021

Comparison Summary

slava77 commented Jul 7, 2021

cmsbuild commented Jul 7, 2021

qliphy commented Jul 7, 2021