[DD4hep] Remove unused PPS materials from Run 3 2021 #36163

cvuosalo · 2021-11-17T23:28:52Z

The file Geometry/VeryForwardData/data/CTPPS_Pixel_2021/Modules/v2/ppstrackerMaterials.xml contains PPS materials that are not used anywhere, as far as I can tell. Some of the composite materials are defined in the wrong order, which could cause problems in the future. It is planned to require that all components of a composite material be defined before the material itself is defined, but we can't make that change as long as there are existing composite materials defined in the wrong order. The PPS materials in this file are the only materials in the Run 3 geometry that are defined in the wrong order. Since they are not even used, it is fairly simple to just remove them.

Another unused material PPS_Epoxy is deleted from the RP_Materials.xml file in this PR.

After this PR, a new big XML file should be created and uploaded to the DB, but that should probably wait until 12_3.

PR validation:

The DD4hep XML workflow, 11634.911, was run successfully with this PR.

No backport is needed.

cvuosalo · 2021-11-17T23:30:29Z

Geometry/VeryForwardGeometry/data/dd4hep/v5/geometryRPFromDD_2021.xml

@@ -0,0 +1,126 @@
+<?xml version="1.0"?>


The only changes in this new version is the removal of the ppstrackerMaterials.xml file and composite material PPS_Epoxy.

cvuosalo · 2021-11-17T23:32:05Z

Geometry/VeryForwardGeometry/python/dd4hep/v4/geometryRPFromDD_2021_cfi.py

@@ -1,7 +1,7 @@
 import FWCore.ParameterSet.Config as cms


I don't quite understand why this Python config is in a versioned directory. I'm not sure if a new v5 directory is required for this new version of the file. Since the change is so minor, maybe no new version directory is needed.

Indeed, I tend to agree that we could keep v4 in this particular case, since this is a matter of removing a spurious line.

Hi @diemort
I think it is a matter of organizing the code. In the same as v2 which is defined as a bug fix, I think we should have v5, instead of edit inside v4. And maybe we just use geometryRPFromDD_2021_cfi.py (outside vN) directly, not just a link to vN.geometryRPFromDD_2021_cfi

@srimanob, using geometryRPFromDD_2021_cfi.py outside vN is more prone to errors in case someone forgets to apply the changes that were made in the latest vN and we avoid duplicating files. I'll submit another PR because other non-dd4hep files have to be updated with v5.

@diemort , will the PR that you are announcing here imply a creation of a Geometry/VeryForwardGeometry/python/dd4hep/v5 area, which will contain a link to a (possibly modified)
Geometry/VeryForwardGeometry/data/dd4hep/v5/geometryRPFromDD_2021.xml?

If so, do you plan to revert Geometry/VeryForwardGeometry/python/dd4hep/v4/geometryRPFromDD_2021_cfi.py to point to Geometry/VeryForwardGeometry/data/dd4hep/v4/geometryRPFromDD_2021.xml?

In that case, it looks to me cleaner and less error prone to introduce a Geometry/VeryForwardGeometry/python/dd4hep/v5 area since now, and modifying it afterwards, so that v4 will remain unchanged by construction.

At the contrary, if you plan to create a brand new Geometry/VeryForwardGeometry/data/dd4hep/v6/geometryRPFromDD_2021.xml, you can link it from the new Geometry/VeryForwardGeometry/python/dd4hep/v5 area (and Geometry/VeryForwardGeometry/python/dd4hep/v4 will link the fixed Geometry/VeryForwardGeometry/data/dd4hep/v5/geometryRPFromDD_2021.xml, as it is now). In this second case we can merge this PR as it is coded now: just let us know.

@perrotta I partially agreed with @srimanob that we should follow the convention and make the changes into a v5, keeping v4 untouched. So I agree with your cleaner suggestion and introduce v5 now. OTOH, I'd still keep the link outside the vN folder pointing to v5, which is the latest geometry description.

Besides, my next PR will update the entry for v5/RP_Materials.xml in Geometry/VeryForwardGeometry/python/v3/geometryRPFromDD_202*.xml (new v3) and update the links at Geometry/VeryForwardGeometry/python/geometryRPFromDD_202*.xml

@diemort I was planning to include your changes in this PR, but if you want to submit a separate PR, that would be fine. I may not be able to make the changes until Monday. I was figuring that there is still time for changes since this PR should wait until it can go into 12_3.

@cvuosalo no problem, I can make a separate PR as soon as this one is merged.

cvuosalo · 2021-11-17T23:33:56Z

@fabferro Please check whether this removal of references to the ppstrackerMaterials.xml file is OK.

cmsbuild · 2021-11-17T23:34:16Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36163/26714

This PR adds an extra 36KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Geometry/python/dict2021Geometry.py modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021.xml modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021FlatMinus05Percent.xml modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021FlatMinus10Percent.xml modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021FlatPlus05Percent.xml modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021FlatPlus10Percent.xml modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021ZeroMaterial.xml modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/python/cmsExtendedGeometry2021FlatMinus05PercentXML_cfi.py modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/python/cmsExtendedGeometry2021FlatMinus10PercentXML_cfi.py modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/python/cmsExtendedGeometry2021FlatPlus05PercentXML_cfi.py modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/python/cmsExtendedGeometry2021FlatPlus10PercentXML_cfi.py modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/python/cmsExtendedGeometry2021XML_cfi.py modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/python/cmsExtendedGeometry2021ZeroMaterialXML_cfi.py modified in PR(s): Python3syntaxupgrade #36160

cmsbuild · 2021-11-17T23:34:21Z

A new Pull Request was created by @cvuosalo (Carl Vuosalo) for master.

It involves the following packages:

Configuration/Geometry (geometry, upgrade)
Geometry/CMSCommonData (geometry, upgrade)
Geometry/VeryForwardData (geometry)
Geometry/VeryForwardGeometry (geometry)

@civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob can you please review it and eventually sign? Thanks.
@missirol, @fabferro, @jan-kaspar, @kpedro88, @Martin-Grunewald, @bsunanda, @vargasa, @fabiocos, @slomeo 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

fabferro · 2021-11-18T07:48:09Z

@diemort please have a look

diemort · 2021-11-18T11:36:50Z

@fabferro @cvuosalo dropping ppstrackerMaterials.xml is OK from my side.

cvuosalo · 2021-11-18T17:56:08Z

There's another unused material whose removal I will be adding to this PR.

cvuosalo · 2021-11-18T18:17:37Z

Geometry/VeryForwardData/data/RP_Materials/v5/RP_Materials.xml

@@ -0,0 +1,68 @@
+<?xml version="1.0"?>


The only change for this file is the deletion of unused composite material PPS_Epoxy.

cmsbuild · 2021-11-18T18:21:01Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36163/26735

This PR adds an extra 40KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Geometry/python/dict2021Geometry.py modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021.xml modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021FlatMinus05Percent.xml modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021FlatMinus10Percent.xml modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021FlatPlus05Percent.xml modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021FlatPlus10Percent.xml modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021ZeroMaterial.xml modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/python/cmsExtendedGeometry2021FlatMinus05PercentXML_cfi.py modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/python/cmsExtendedGeometry2021FlatMinus10PercentXML_cfi.py modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/python/cmsExtendedGeometry2021FlatPlus05PercentXML_cfi.py modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/python/cmsExtendedGeometry2021FlatPlus10PercentXML_cfi.py modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/python/cmsExtendedGeometry2021XML_cfi.py modified in PR(s): Python3syntaxupgrade #36160
- File Geometry/CMSCommonData/python/cmsExtendedGeometry2021ZeroMaterialXML_cfi.py modified in PR(s): Python3syntaxupgrade #36160

cmsbuild · 2021-11-18T18:21:27Z

Pull request #36163 was updated. @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob can you please check and sign again.

cvuosalo · 2021-11-18T18:23:06Z

@cmsbuild please test

cmsbuild · 2021-11-18T21:24:19Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0a12ff/20619/summary.html
COMMIT: 2b92f71
CMSSW: CMSSW_12_2_X_2021-11-18-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36163/20619/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 8 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 3327156
DQMHistoTests: Total failures: 11
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3327122
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 41 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

cmsbuild · 2021-11-24T08:49:42Z

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-24T15:31:13Z

@cvuosalo if the merging of this (rather technical) PR has to wait for 12_3, then better to put it on hold till next week, and perhaps also write somehow "12_3_X" in the title, so that it is crystal clear

cvuosalo · 2021-11-24T19:13:18Z

@perrotta This PR makes a minor, technical change to the geometry, but it does change the geometry nonetheless. For consistency, the DD4hep geometry in the DB should also be changed to match, which would mean uploading a new big XML file to the DB and creating a new Global Tag. I was thinking all these steps should wait for 12_3.

If you want to merge this PR into 12_2, I suppose that would be OK, though it would introduce a trivial inconsistency between the DD4hep XML and DB geometry in 12_2 that would be rectified in 12_3.

cvuosalo · 2021-11-30T17:07:23Z

hold

cmsbuild · 2021-11-30T17:07:49Z

Pull request has been put on hold by @cvuosalo
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

cvuosalo · 2021-11-30T17:09:51Z

I'm going to include the geometryRPFromDD_2021_cfi.py changes in this PR in the next few hours and then it can be merged.

updating DD files with v5 after removing PPS materials. First step. Will require more revision

cmsbuild · 2021-11-30T17:21:57Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36163/27032

This PR adds an extra 44KB to repository

cmsbuild · 2021-11-30T17:22:18Z

Pull request #36163 was updated. @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob can you please check and sign again.

cvuosalo · 2021-11-30T20:03:43Z

Closing in favor of #36307.

[DD4hep] Remove unused PPS materials from Run 3 2021 (replaces #36163)

cvuosalo · 2022-03-21T19:10:31Z

-1

Remove unused PPS materials

5b6c048

cmsbuild added this to the CMSSW_12_2_X milestone Nov 17, 2021

cmsbuild added code-checks-pending geometry-pending orp-pending pending-signatures tests-pending upgrade-pending labels Nov 17, 2021

cvuosalo commented Nov 17, 2021

View reviewed changes

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

Also remove unused PPS_Epoxy

2b92f71

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

cvuosalo commented Nov 18, 2021

View reviewed changes

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

cmsbuild added tests-started and removed tests-pending labels Nov 18, 2021

cmsbuild added the upgrade-approved label Nov 24, 2021

cmsbuild added hold pending-signatures and removed fully-signed labels Nov 30, 2021

Merge pull request #1 from diemort/rm-unused-PPS-materials

fa5d0e8

updating DD files with v5 after removing PPS materials. First step. Will require more revision

cmsbuild added code-checks-pending geometry-pending tests-pending upgrade-pending and removed geometry-approved tests-approved upgrade-approved code-checks-approved labels Nov 30, 2021

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

cvuosalo mentioned this pull request Nov 30, 2021

[DD4hep] Remove unused PPS materials from Run 3 2021 (replaces #36163) #36307

Merged

cvuosalo closed this Nov 30, 2021

cmsbuild added a commit that referenced this pull request Dec 1, 2021

Merge pull request #36307 from cvuosalo/pull/36163

8fa87a9

[DD4hep] Remove unused PPS materials from Run 3 2021 (replaces #36163)

cmsbuild added geometry-rejected and removed geometry-pending labels Mar 21, 2022

[DD4hep] Remove unused PPS materials from Run 3 2021 #36163

[DD4hep] Remove unused PPS materials from Run 3 2021 #36163

Uh oh!

Conversation

cvuosalo commented Nov 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR validation:

Uh oh!

cvuosalo Nov 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cvuosalo commented Nov 17, 2021

Uh oh!

cmsbuild commented Nov 17, 2021

Uh oh!

cmsbuild commented Nov 17, 2021

Uh oh!

fabferro commented Nov 18, 2021

Uh oh!

diemort commented Nov 18, 2021

Uh oh!

cvuosalo commented Nov 18, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmsbuild commented Nov 18, 2021

Uh oh!

cmsbuild commented Nov 18, 2021

Uh oh!

cvuosalo commented Nov 18, 2021

Uh oh!

cmsbuild commented Nov 18, 2021

Comparison Summary

Uh oh!

cmsbuild commented Nov 24, 2021

Uh oh!

perrotta commented Nov 24, 2021

Uh oh!

cvuosalo commented Nov 24, 2021

Uh oh!

cvuosalo commented Nov 30, 2021

Uh oh!

cmsbuild commented Nov 30, 2021

Uh oh!

cvuosalo commented Nov 30, 2021

Uh oh!

cmsbuild commented Nov 30, 2021

Uh oh!

cmsbuild commented Nov 30, 2021

Uh oh!

cvuosalo commented Nov 30, 2021

Uh oh!

cvuosalo commented Mar 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

cvuosalo commented Nov 17, 2021 •

edited

Loading

cvuosalo Nov 17, 2021 •

edited

Loading