New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add evaluation of SpecPar numeric values in Phase II Tracker XMLs #32479
Conversation
… XMLs. This will help speed-up the DD4hep workflows (no time lost with string handling).
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32479/20385
|
A new Pull Request was created by @ghugo83 for master. It involves the following packages: Geometry/TrackerCommonData @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild please test |
Should any of the values be given units? For example, is |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3eef8b/11644/summary.html Comparison SummarySummary:
|
@cvuosalo No it is fine, all of these are without unit. |
@ghugo83 - the way all these specpars are defined looks quite wasteful: thousands of repetitive lines and identical constants... I'd be surprised if a specpar name is used anywhere. I'd suggest we optimise these descriptions. |
@ianna I am looking at Run 3 for now, I have found ways of speeding up Run 3 DD4hep workflow very significantly :) Will PR it today / tomorrow. Regarding the performance of dd4hep workflow in general: Regarding these XMLs: the PixelROCRows constants blocks are indeed repeated for each sensor. I already plan to have a set of Phase 2 XMLs generated for DD4hep workflow anyway, at least for testing purposes, to re-arrange the materials in an order which works in the DD4hep case. |
+1 |
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) |
+1 |
This could help speeding-up the DD4hep workflows.
A recent change was added to the dd4hep-version of the FilteredView in CMSSW, in order to directly handle numerical values from XMLs, instead of first storing them as strings.
Mentioned in #32414
@ianna
NB 1: Sim And Reco Phase II XMLs already had evaluated numerical values, nothing to do there.
NB 2: I noticed that there are other XML files with non-evaluated numeric values, outside of Phase II
(except Geometry/TrackerCommonData/data/PhaseI/trackerStructureTopology.xml, which was already fixed in 32a0c74):
Geometry/TrackerCommonData/data/trackerStructureTopology.xml
Geometry/TrackerCommonData/data/Pilot/trackerStructureTopology.xml
Geometry/TrackerCommonData/data/CRack/trackerStructureTopology.xml
Geometry/TwentyFivePercentTrackerCommonData/data/trackerStructureTopology_twentyfivepercent.xml
Geometry/TrackerCommonData/data/PhaseI/PixelForward/trackerStructureTopology.xml
Is it worth it modifying them as well, as they are either unused or pre-Run 3 ? I assume it is not.