Skip to content

Conversation

@ianna
Copy link
Contributor

@ianna ianna commented Apr 27, 2021

PR description:

@cvuosalo - IMHO, this is a very straightforward way: if xml_geometry_payload is defined, a big XML will be produced (std::cout for now).

Please, see the test:

cmsRun DetectorDescription/DDCMS/test/python/testShapes.py

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:

@ianna
Copy link
Contributor Author

ianna commented Apr 27, 2021

@cvuosalo - please, see 5539cc7

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33548/22329

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

@perrotta
Copy link
Contributor

-1
(please rebase in order to get rid of unwanted commits)

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33548/23350

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

Calibration/EcalCalibAlgos
DetectorDescription/DDCMS
Geometry/MTDCommonData
RecoLocalTracker/SiPixelRecHits

@perrotta, @malbouis, @civanch, @Dr15Jones, @makortel, @cvuosalo, @tlampen, @ianna, @kpedro88, @cmsbuild, @srimanob, @yuanchao, @mdhildreth, @slava77, @jpata, @pohsun, @francescobrivio can you please review it and eventually sign? Thanks.
@felicepantaleo, @argiro, @OzAmram, @thomreis, @threus, @slomeo, @vargasa, @makortel, @JanFSchulte, @simonepigazzini, @GiacomoSguazzoni, @rovere, @VinInn, @ferencek, @tocheng, @mmusich, @mtosi, @fabiocos, @rchatter, @dkotlins, @gpetruc, @tvami 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

// specific to PixelCPEFastESProducer
desc.add<std::string>("ComponentName", "PixelCPEFast");
desc.add<edm::ESInputTag>("MagneticFieldRecord", edm::ESInputTag());
desc.addOptional<bool>("useLAAlignmentOffsets", false)->setComment("deprecated");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ianna why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think rebase didn't take into an account reverted changes :-(

// specific to PixelCPEGenericESProducer
desc.add<std::string>("ComponentName", "PixelCPEGeneric");
desc.add<edm::ESInputTag>("MagneticFieldRecord", edm::ESInputTag(""));
desc.addOptional<bool>("useLAAlignmentOffsets", false)->setComment("deprecated");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ianna why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think rebase didn't take into an account reverted changes:


Revert "Update mtdParameters for D49 and D60 (Bug fix)", This reverts…  … 1307d75

Revert "[backport] MTD geometry and reconstruction: synchronise with …  … faf18ac

change ECAL pedestals requireStableBeam flag for MWGR test        2a9cbcd

Revert "Revert "[backport] MTD geometry and reconstruction: synchroni…  …f2a994f

revert changes breaking online HLT menu in production release         93afbed


// specific to PixelCPETemplateRecoESProducer
desc.add<std::string>("ComponentName", "PixelCPETemplateReco");
desc.addOptional<bool>("DoLorentz", true)->setComment("deprecated");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ianna why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same reason as above

@fabiocos
Copy link
Contributor

@ianna why restoring old parameters in mtdParameters.xml?

@fabiocos
Copy link
Contributor

I guess it is a similar issue as in the discussion with @mmusich ... There is no reason to go back anyway

@ianna
Copy link
Contributor Author

ianna commented Jun 16, 2021

I guess it is a similar issue as in the discussion with @mmusich ... There is no reason to go back anyway

Yes, I have no idea why a rebase picked the other commits. I'll try to rebase again a bit later. If this will not work, I'll cherry-pick my commit and open a new PR.

@civanch
Copy link
Contributor

civanch commented Jun 16, 2021

@ianna , better to start a new PR even if extra work will be needed.

@slava77
Copy link
Contributor

slava77 commented Jun 17, 2021

-1

it looks like the rebase went wrong and changes in reco code are not expected

@srimanob
Copy link
Contributor

srimanob commented Jul 8, 2021

@ianna
Do we still need this, or should it be closed? Thanks.

@ianna ianna closed this Jul 9, 2021
@ianna ianna deleted the dd4hep-geometry-payload branch June 25, 2025 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants