Skip to content
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

Update of the RPC migration to DD4hep #30047

Closed
wants to merge 10 commits into from

Conversation

slomeo
Copy link
Contributor

@slomeo slomeo commented May 29, 2020

PR description:

Update the RPC migration to DD4hep after @bsunanda PRs #29842 #29943 and @ianna PR #29954.

PR validation:

  1. validation made by Geometry/RPCGeometryBuilder/test/python/validateRPCGeometry_cfg.py

  2. validation by runTheMatrix.py -l 25202.1

25202.1_TTbar_13+TTbar_13+DIGIUP15APVSimu_PU25+RECOUP15_PU25+HARVESTUP15_PU25 Step0-PASSED Step1-PASSED Step2-PASSED Step3-PASSED - time date Fri May 29 17:36:47 2020-date Fri May 29 17:15:13 2020; exit: 0 0 0 0
1 1 1 1 tests passed, 0 0 0 0 failed

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

nothing special

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30047/15752

  • This PR adds an extra 20KB to repository

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

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30047/15753

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @slomeo (Sergio Lo Meo) for master.

It involves the following packages:

Geometry/RPCGeometryBuilder
RecoLocalMuon/GEMRecHit

@perrotta, @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @kpedro88, @slava77 can you please review it and eventually sign? Thanks.
@jshlee, @bellan, @fabiocos, @jhgoh this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

@kpedro88
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 29, 2020

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/6694/console Started: 2020/05/29 18:31

@cmsbuild
Copy link
Contributor

+1
Tested at: c2b0d2d
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3104a3/6694/summary.html
CMSSW: CMSSW_11_2_X_2020-05-29-1100
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30047/15854

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2020

Pull request #30047 was updated. @perrotta, @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @kpedro88, @slava77 can you please check and sign again.

@perrotta
Copy link
Contributor

perrotta commented Jun 4, 2020

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2020

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/6802/console Started: 2020/06/04 11:32

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2020

-1

Tested at: b2790b1

CMSSW: CMSSW_11_2_X_2020-06-03-2300
SCRAM_ARCH: slc7_amd64_gcc820
You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3104a3/6802/summary.html

I found follow errors while testing this PR

Failed tests: UnitTests

  • Unit Tests:

I found errors in the following unit tests:

---> test testDD4hepFilteredView had ERRORS
---> test DetectorDescriptionDDCMSTestDriver had ERRORS

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2020

Comparison job queued.

@slomeo
Copy link
Contributor Author

slomeo commented Jun 4, 2020

@ianna @cvuosalo looking at https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3104a3/6802/summary.html it seems that there is a problem related to cms-geometry-2021.xml. I deleted this file so I think it is necessary to update some scripts. Am I correct? If I'm correct, can you please take care of it?

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 4, 2020

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 7462 differences found in the comparisons
  • DQMHistoTests: Total files compared: 36
  • DQMHistoTests: Total histograms compared: 2783946
  • DQMHistoTests: Total failures: 17033
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2766863
  • DQMHistoTests: Total skipped: 50
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 35 files compared)
  • Checked 152 log files, 16 edm output root files, 36 DQM output files

@ianna
Copy link
Contributor

ianna commented Jun 4, 2020

@ianna @cvuosalo looking at https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3104a3/6802/summary.html it seems that there is a problem related to cms-geometry-2021.xml. I deleted this file so I think it is necessary to update some scripts. Am I correct? If I'm correct, can you please take care of it?

@slomeo - what I was asking is to remove your changes to Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021.xml and Configuration/Geometry/python/GeometryDD4hepExtended2021_cff.py in this PR.

@@ -5,7 +5,7 @@
#
# Ideal geometry, needed for simulation
DDDetectorESProducer = cms.ESSource("DDDetectorESProducer",
confGeomXMLFiles = cms.FileInPath('DetectorDescription/DDCMS/data/cms-geometry-2021.xml'),
confGeomXMLFiles = cms.FileInPath('Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021.xml'),
Copy link
Contributor

Choose a reason for hiding this comment

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

please, remove modifications to this file from your PR

@@ -1,308 +0,0 @@
<?xml version="1.0"?>
Copy link
Contributor

Choose a reason for hiding this comment

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

please, do not delete it in this PR

@@ -4,286 +4,286 @@
<close_geometry/>

<IncludeSection>
<Include ref="Geometry/CMSCommonData/data/materials/2021/v1/materials.xml"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

please, remove modifications to this file from this PR

@slomeo
Copy link
Contributor Author

slomeo commented Jun 4, 2020

@ianna do you mean a "git restore"?

@kpedro88
Copy link
Contributor

kpedro88 commented Jun 4, 2020

@slomeo 'git restore' is not the right command. You should 'git revert' the commit in which you removed the file, additionally undo any changes you previously made to the files mentioned by @ianna, and then squash the commits in this PR to avoid churn in the history and blame.

@slomeo
Copy link
Contributor Author

slomeo commented Jun 5, 2020

@ianna @kpedro88 I think it's better to re-start with a new PR in a clean IB. I'll close this PR and I'll make a new PR asap.

@slomeo slomeo closed this Jun 5, 2020
@slomeo
Copy link
Contributor Author

slomeo commented Jun 5, 2020

I made a new PR, please look to #30121

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.

8 participants