DD4hep: Ecal Barrel New Algorithm v2 #27444

ianna · 2019-07-04T16:21:16Z

PR description:

Complete migration of a DDEcalBarrelNewAlgo class ASIS

Note: there are known issues which will be listed and discussed during Sim meeting: units, CLHEP, code modernisation, etc. Please, open an issue for other comments after that discussion. The migrated class is quite lengthy (over 2000 lines) and is needed ASIS for identity tests.

PR validation:

cmsRun Geometry/EcalAlgo/test/python/testEcalGeometry.py
cmsShow -c ecal.fwc --tgeo-name Ecal --sim-geom-file cmsDD4HepGeom.root

Baseline Ecal barrel has 37 overlaps:

These 37 were carried over to the DD4hep version:

Fully implemented Ecal barrel geometry description compared with DD one:

if this PR is a backport please specify the original PR:

Before submitting your pull requests, make sure you followed this checklist:

verify that the PR is really intended for the chosen branch
verify that changes follow CMS Naming, Coding, And Style Rules
verify that the PR passes the basic test procedure suggested in the CMSSW PR instructions

ianna · 2019-07-04T16:21:32Z

please test

cmsbuild · 2019-07-04T16:23:32Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-07-04T16:31:27Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27444/10753

This PR adds an extra 100KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/Math/interface/GeantUnits.h modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/BuildFile.xml modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/DDEcalBarrelAlgo.cc modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/DDEcalBarrelNewAlgo.cc modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/DDEcalEndcapAlgo.cc modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/DDEcalEndcapTrap.cc modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/DDEcalEndcapTrap.h modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/DDEcalPreshowerAlgo.cc modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/dd4hep/DDEcalBarrelNewAlgo.cc modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402

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-27444/10753/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27444/10753/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2019-07-04T16:42:31Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-07-04T16:50:03Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27444/10754

This PR adds an extra 124KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/Math/interface/GeantUnits.h modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/BuildFile.xml modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/DDEcalBarrelAlgo.cc modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/DDEcalBarrelNewAlgo.cc modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/DDEcalEndcapAlgo.cc modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/DDEcalEndcapTrap.cc modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/DDEcalEndcapTrap.h modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/DDEcalPreshowerAlgo.cc modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402, Ecal Geometry: Algorithms Migration from CLHEP to GeantUnits #27395
- File Geometry/EcalCommonData/plugins/dd4hep/DDEcalBarrelNewAlgo.cc modified in PR(s): DD4Hep: Ecal Barrel (new) Algorithm #27402

cmsbuild · 2019-07-04T16:50:31Z

A new Pull Request was created by @ianna (Ianna Osborne) for master.

It involves the following packages:

DataFormats/Math
DetectorDescription/DDCMS
Geometry/EcalAlgo
Geometry/EcalCommonData

@perrotta, @civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @rovere, @argiro this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

ianna · 2019-07-04T17:02:54Z

please test

cmsbuild · 2019-07-04T17:03:21Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1330/console Started: 2019/07/04 19:03

cmsbuild · 2019-07-04T17:13:58Z

-1

Tested at: 785e5df

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f80636/1330/summary.html

I found follow errors while testing this PR

Failed tests: Build ClangBuild

Build:

I found compilation warning when building: See details on the summary page.

Clang:

I found compilation warning while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

See details on the summary page.

cmsbuild · 2019-07-04T17:14:00Z

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

cmsbuild · 2019-07-08T14:07:23Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-07-08T14:14:11Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27444/10776

This PR adds an extra 208KB to repository

cmsbuild · 2019-07-08T14:14:41Z

Pull request #27444 was updated. @perrotta, @civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @slava77 can you please check and sign again.

ianna · 2019-07-08T14:23:08Z

please test

cmsbuild · 2019-07-08T14:23:31Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1360/console Started: 2019/07/08 16:25

cmsbuild · 2019-07-08T16:33:02Z

+1
Tested at: 33c0fc7
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f80636/1360/summary.html

cmsbuild · 2019-07-08T16:33:06Z

Comparison job queued.

ianna · 2019-07-08T16:35:04Z

+1

cmsbuild · 2019-07-08T19:06:29Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 33
DQMHistoTests: Total histograms compared: 3160399
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3160063
DQMHistoTests: Total skipped: 334
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
Checked 137 log files, 14 edm output root files, 33 DQM output files

fabiocos · 2019-07-09T04:52:00Z

@slava77 @perrotta do you have any objection? Or could we otherwise move this PR forward?

slava77 · 2019-07-09T15:40:29Z

+1

for #27444 33c0fc7

code changes in the reco category are only in DataFormats/Math/interface/GeantUnits.h with a somewhat trivial addition of literal operators
jenkins tests pass and comparisons with the baseline show no differences

cmsbuild · 2019-07-09T15:41:03Z

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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

fabiocos · 2019-07-09T17:44:39Z

+1

cmsbuild added this to the CMSSW_11_0_X milestone Jul 4, 2019

cmsbuild added code-checks-pending comparison-pending geometry-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Jul 4, 2019

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

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

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

cmsbuild added comparison-notrun tests-rejected and removed comparison-pending tests-started labels Jul 4, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 8, 2019

cmsbuild added tests-started and removed tests-pending labels Jul 8, 2019

cmsbuild added tests-approved and removed tests-started labels Jul 8, 2019

cmsbuild added geometry-approved and removed geometry-pending labels Jul 8, 2019

cmsbuild added comparison-available and removed comparison-pending labels Jul 8, 2019

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Jul 9, 2019

cmsbuild added orp-approved and removed orp-pending labels Jul 9, 2019

cmsbuild merged commit 75cc942 into cms-sw:master Jul 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DD4hep: Ecal Barrel New Algorithm v2 #27444

DD4hep: Ecal Barrel New Algorithm v2 #27444

ianna commented Jul 4, 2019 •

edited

ianna commented Jul 4, 2019

cmsbuild commented Jul 4, 2019

cmsbuild commented Jul 4, 2019

cmsbuild commented Jul 4, 2019

cmsbuild commented Jul 4, 2019

cmsbuild commented Jul 4, 2019

ianna commented Jul 4, 2019

cmsbuild commented Jul 4, 2019 •

edited

cmsbuild commented Jul 4, 2019

cmsbuild commented Jul 4, 2019

cmsbuild commented Jul 8, 2019

cmsbuild commented Jul 8, 2019

cmsbuild commented Jul 8, 2019

ianna commented Jul 8, 2019

cmsbuild commented Jul 8, 2019 •

edited

cmsbuild commented Jul 8, 2019

cmsbuild commented Jul 8, 2019

ianna commented Jul 8, 2019

cmsbuild commented Jul 8, 2019

fabiocos commented Jul 9, 2019

slava77 commented Jul 9, 2019

cmsbuild commented Jul 9, 2019

fabiocos commented Jul 9, 2019

DD4hep: Ecal Barrel New Algorithm v2 #27444

DD4hep: Ecal Barrel New Algorithm v2 #27444

Conversation

ianna commented Jul 4, 2019 • edited

PR description:

PR validation:

if this PR is a backport please specify the original PR:

ianna commented Jul 4, 2019

cmsbuild commented Jul 4, 2019

cmsbuild commented Jul 4, 2019

cmsbuild commented Jul 4, 2019

cmsbuild commented Jul 4, 2019

cmsbuild commented Jul 4, 2019

ianna commented Jul 4, 2019

cmsbuild commented Jul 4, 2019 • edited

cmsbuild commented Jul 4, 2019

cmsbuild commented Jul 4, 2019

cmsbuild commented Jul 8, 2019

cmsbuild commented Jul 8, 2019

cmsbuild commented Jul 8, 2019

ianna commented Jul 8, 2019

cmsbuild commented Jul 8, 2019 • edited

cmsbuild commented Jul 8, 2019

cmsbuild commented Jul 8, 2019

ianna commented Jul 8, 2019

cmsbuild commented Jul 8, 2019

fabiocos commented Jul 9, 2019

slava77 commented Jul 9, 2019

cmsbuild commented Jul 9, 2019

fabiocos commented Jul 9, 2019

ianna commented Jul 4, 2019 •

edited

cmsbuild commented Jul 4, 2019 •

edited

cmsbuild commented Jul 8, 2019 •

edited