DD4hep: add an algorithm needed to build MTD geometry

[ianna]

PR description:

• Add DDTrackerRingAlgo needed to build MTD geometry

@vargasa - please, check the algorithm implementation.

PR validation:

cmsRun Geometry/MTDGeometryBuilder/test/python/testMTDGeometry.py
cmsShow -c geo.fwc --sim-geom-file cmsDD4HepGeom.root --tgeo-name=MTD

Geometry description: left - DD CMS, right - DD4hep:

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

no back port is needed

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26386/9137

• This PR adds an extra 52KB to repository

[cmsbuild]

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

It involves the following packages:

DetectorDescription/DDCMS
Geometry/EcalAlgo
Geometry/EcalCommonData
Geometry/MTDCommonData
Geometry/MTDGeometryBuilder
Geometry/TrackerCommonData
Geometry/TrackerGeometryBuilder

@civanch, @Dr15Jones, @cvuosalo, @ianna, @kpedro88, @cmsbuild, @mdhildreth can you please review it and eventually sign? Thanks.
@vargasa, @makortel, @argiro, @ghugo83, @ebrondol, @VinInn, @venturia 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]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/34055/console Started: 2019/04/08 16:48

[ianna]

@vargasa - I would appreciate if you could scrutinize the DDTrackerRingAlgo algorithm and propose the most optimized version.

The old version was registering all rotations by name and (possibly) reusing them in order to save space. This version does not register the rotations, though it can be done by querying the cache:

    string tiltRotstr = rotstr + "Tilt" + to_string(convertRadToDeg(tiltAngle)) + "ZMinus";
    tiltRot = ns.rotation(tiltRotstr);

and if there is no such rotation - an Identity matrix is returned in this case - adding it to the cache:

   tiltRot = makeRotation3D(90._deg, 90._deg, tiltAngle, 0._deg, 90._deg + tiltAngle, 0._deg);
   ns.addRotation(tiltRotstr, tiltRot);

This could save space, but would add time on string manipulation, search, comparison and updating the map. However, it would be nice to profile it.

Another question if some these matrices are redundant and can be deleted:

  Rotation3D flipRot, tiltRot, phiRot, globalRot; // Identity
  Rotation3D flipMatrix, tiltMatrix, phiRotMatrix, globalRotMatrix; // Identity matrix

Thanks.

[cmsbuild]

+1
Tested at: 61aa9fd
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-26386/34055/summary.html

[cmsbuild]

Comparison job queued.

[ianna]

+1

[cmsbuild]

+1
Tested at: 8e6f64b
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-26386/34070/summary.html

[cmsbuild]

Comparison job queued.

[ianna]

+1

[cmsbuild]

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

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 5 differences found in the comparisons
• DQMHistoTests: Total files compared: 32
• DQMHistoTests: Total histograms compared: 3140495
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3140297
• DQMHistoTests: Total skipped: 197
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
• Checked 133 log files, 14 edm output root files, 32 DQM output files

[vargasa]

@ianna thanks a lot for this info. I will check.

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

[ianna]

@fabiocos - please, merge this PR so that I could close the others

[fabiocos]

@ianna this is effectively embedding #26373 and #26374 . I will merge this for next IB, the others may be closed

[fabiocos]

+1

@vargasa we may follow separately for possible further code improvements

[ianna]

@vargasa - FYI, @ghugo83 has suggested the algorithm can be generalised and used by other sub-detectors. Please, discuss it with her.

[cvuosalo]

I don't think this line is necessary. Maybe it can be fixed in some later PR.