[DD4hep] use DD4hep consistently in configurations #36004

ianna · 2021-11-05T10:14:13Z

PR description:

address issue #33373

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that 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 · 2021-11-05T10:14:30Z

please test

cmsbuild · 2021-11-05T10:21:16Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36004/26439

This PR adds an extra 248KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Alignment/OfflineValidation/plugins/TrackerGeometryCompare.cc modified in PR(s): Migrate to the new ref. based writeOneIOV() in Alignment package #35711

cmsbuild · 2021-11-05T10:21:44Z

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

It involves the following packages:

Alignment/MuonAlignment (alca)
Alignment/OfflineValidation (alca)
CondTools/DT (db)
CondTools/Geometry (db)
Geometry/CMSCommonData (geometry, upgrade)
Geometry/CaloEventSetup (geometry)
Geometry/DTGeometry (geometry)
Geometry/DTGeometryBuilder (geometry)
Geometry/EcalAlgo (geometry)
Geometry/EcalCommonData (geometry)
Geometry/ForwardCommonData (geometry)
Geometry/GEMGeometryBuilder (geometry, upgrade)
Geometry/HGCalCommonData (geometry, upgrade)
Geometry/HcalAlgo (geometry)
Geometry/HcalCommonData (geometry)
Geometry/HcalTestBeamData (geometry)
Geometry/HcalTowerAlgo (geometry)
Geometry/MuonNumbering (geometry)
Geometry/RPCGeometryBuilder (geometry)
Geometry/TrackerGeometryBuilder (geometry)
Geometry/VeryForwardGeometryBuilder (geometry)
SimG4Core/Application (simulation)
SimG4Core/Configuration (simulation)
SimG4Core/Geometry (simulation)
SimG4Core/GeometryProducer (simulation)
SimG4Core/PrintGeomInfo (simulation)

@malbouis, @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @AdrianoDee, @srimanob, @yuanchao, @ggovi, @tvami, @francescobrivio can you please review it and eventually sign? Thanks.
@abbiendi, @CeliaFernandez, @argiro, @jshlee, @bsunanda, @jhgoh, @thomreis, @tlampen, @slomeo, @venturia, @pakhotin, @battibass, @makortel, @JanFSchulte, @watson-ij, @simonepigazzini, @trocino, @cvuosalo, @fabferro, @rovere, @VinInn, @dildick, @tocheng, @mmusich, @ptcox, @mtosi, @fabiocos, @adewit, @rchatter, @jan-kaspar this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

cmsbuild · 2021-11-05T14:03:50Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7e7a6c/20273/summary.html
COMMIT: 8cea020
CMSSW: CMSSW_12_2_X_2021-11-04-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36004/20273/install.sh to create a dev area with all the needed externals and cmssw changes.

CMS StaticAnalyzer warnings: There are 2 EventSetupRecord::get warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7e7a6c/20273/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: 42
DQMHistoTests: Total histograms compared: 2901890
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2901868
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 41 files compared)
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

tvami · 2021-11-05T14:08:34Z

+1

tests pass
please note the the ES get() migration still needs to be done here, see https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7e7a6c/20273/llvm-analysis/esrget-sa.txt

srimanob · 2021-11-05T14:31:47Z

+Upgrade

Technical PR to resolve the issue of mixing between DD4hep and else (DD4Hep,...). No change is expected.

civanch · 2021-11-05T16:09:54Z

+1

cmsbuild · 2021-11-05T16:10:19Z

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

perrotta · 2021-11-05T16:17:20Z

+1

Geometry/EcalAlgo/plugins/BuildFile.xml

cvuosalo · 2021-11-05T20:05:17Z

The file Geometry/CSCGeometryBuilder/test/python/validateCSCGeometryDD4hep_cfg.py contains the string validateCSCGeometryDD4HEP.root.
SimG4CMS/HGCalTestBeam/test/HGCalTBCERN181Oct1DD4Hep_cfg.py contains the strings TBGenSim181Oct1DD4Hep.root and TBGenSimDD4Hep.root.

Should they be changed for consistency?

cvuosalo · 2021-11-05T22:03:46Z

Geometry/HcalTestBeamData/interface/HcalTB02ParametersFromDD.h

@@ -18,7 +18,7 @@ class HcalTB02ParametersFromDD {

 private:
  static constexpr double k_ScaleFromDDDToG4 = 1.0;
-  static constexpr double k_ScaleFromDD4HepToG4 = 10.0;
+  static constexpr double k_ScaleFromDD4hepToG4 = 10.0;


@bsunanda Not related to this PR, but this value looks wrong. This file may have been missed in the units conversion. I think it should be:

static constexpr double k_ScaleFromDD4hepToG4 = (1.0 / dd4hep::mm);

OK Carl I shall take care of this

cvuosalo · 2021-11-05T22:32:01Z

Doing git grep DD4Hep with this PR reveals about 20 files that still contain DD4Hep.

perrotta · 2021-11-06T07:15:21Z

Doing git grep DD4Hep with this PR reveals about 20 files that still contain DD4Hep.

Moreover: with lxr I still can find 53 files with "DD4Hep" in the name:
https://cmssdt.cern.ch/lxr/search?%21v=CMSSW_12_2_X_2021-11-05-2300&_filestring=DD4Hep&_string=&_casesensitive=1
and 66 occurrencies from 33 different files with "DD4Hep" in the body:
https://cmssdt.cern.ch/lxr/search?%21v=CMSSW_12_2_X_2021-11-05-2300&_filestring=DD4Hep&_string=&_casesensitive=1

Noteworthy, there are also two files in \test with "DD4HEP" in full capital letters in their names:
https://cmssdt.cern.ch/lxr/search?%21v=CMSSW_12_2_X_2021-11-05-2300&_filestring=DD4HE&_string=&_casesensitive=1
but I think they can stay as such, those names following some reasonable convention

ianna · 2021-11-06T08:46:00Z

Doing git grep DD4Hep with this PR reveals about 20 files that still contain DD4Hep.

@cvuosalo - thanks! I'll have another go at CMSSW now that it is merged. There were way too many hits before. Thanks!

ianna · 2021-11-06T08:49:06Z

There are around 70 places where DD4Hep is mentioned: https://cmssdt.cern.ch/lxr/search?%21v=CMSSW_12_2_X_2021-11-05-2300&_filestring=&_string=DD4Hep&_casesensitive=1

plus a few DD4HEP notations: https://cmssdt.cern.ch/lxr/search?%21v=CMSSW_12_2_X_2021-11-05-2300&_filestring=&_string=DD4HEP&_casesensitive=1

I'll take care of it in a subsequent PR.

ianna · 2021-11-06T14:42:05Z

@cvuosalo and @perrotta - I hope, all your comments are addressed. Please, see #36021

ianna added 2 commits November 5, 2021 11:10

address issue 33373

806de2f

rename DT geometry builder

8cea020

cmsbuild added this to the CMSSW_12_2_X milestone Nov 5, 2021

cmsbuild added alca-pending code-checks-pending db-pending geometry-pending orp-pending pending-signatures simulation-pending tests-started upgrade-pending labels Nov 5, 2021

ianna mentioned this pull request Nov 5, 2021

Mix use of DD4hep and DD4Hep across CMSSW. #33373

Closed

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

tvami mentioned this pull request Nov 5, 2021

[DD4hep] [CMSSW_12_1] use DD4hep consistently in configurations - back port to 12_1 #36003

Closed

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

cmsbuild added alca-approved db-approved and removed alca-pending db-pending labels Nov 5, 2021

cmsbuild added upgrade-approved and removed upgrade-pending labels Nov 5, 2021

cmsbuild added fully-signed geometry-approved simulation-approved and removed simulation-pending geometry-pending pending-signatures labels Nov 5, 2021

cmsbuild added orp-approved and removed orp-pending labels Nov 5, 2021

cmsbuild merged commit 7439de3 into cms-sw:master Nov 5, 2021

cvuosalo reviewed Nov 5, 2021

View reviewed changes

Geometry/EcalAlgo/plugins/BuildFile.xml Show resolved Hide resolved

cvuosalo reviewed Nov 5, 2021

View reviewed changes

ianna mentioned this pull request Nov 6, 2021

[DD4hep] cleanup file names #36021

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DD4hep] use DD4hep consistently in configurations #36004

[DD4hep] use DD4hep consistently in configurations #36004

ianna commented Nov 5, 2021

ianna commented Nov 5, 2021

cmsbuild commented Nov 5, 2021

cmsbuild commented Nov 5, 2021

cmsbuild commented Nov 5, 2021

tvami commented Nov 5, 2021

srimanob commented Nov 5, 2021

civanch commented Nov 5, 2021

cmsbuild commented Nov 5, 2021

perrotta commented Nov 5, 2021

cvuosalo commented Nov 5, 2021 •

edited

cvuosalo Nov 5, 2021

bsunanda Nov 5, 2021

cvuosalo commented Nov 5, 2021

perrotta commented Nov 6, 2021

ianna commented Nov 6, 2021

ianna commented Nov 6, 2021 •

edited

ianna commented Nov 6, 2021

[DD4hep] use DD4hep consistently in configurations #36004

[DD4hep] use DD4hep consistently in configurations #36004

Conversation

ianna commented Nov 5, 2021

PR description:

PR validation:

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

ianna commented Nov 5, 2021

cmsbuild commented Nov 5, 2021

cmsbuild commented Nov 5, 2021

cmsbuild commented Nov 5, 2021

Comparison Summary

tvami commented Nov 5, 2021

srimanob commented Nov 5, 2021

civanch commented Nov 5, 2021

cmsbuild commented Nov 5, 2021

perrotta commented Nov 5, 2021

cvuosalo commented Nov 5, 2021 • edited

cvuosalo Nov 5, 2021

Choose a reason for hiding this comment

bsunanda Nov 5, 2021

Choose a reason for hiding this comment

cvuosalo commented Nov 5, 2021

perrotta commented Nov 6, 2021

ianna commented Nov 6, 2021

ianna commented Nov 6, 2021 • edited

ianna commented Nov 6, 2021

cvuosalo commented Nov 5, 2021 •

edited

ianna commented Nov 6, 2021 •

edited