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

Fix dtSpecsFilter XML used by Phase-2 wf for DD4hep #37078

Merged
merged 1 commit into from Mar 2, 2022

Conversation

srimanob
Copy link
Contributor

@srimanob srimanob commented Feb 27, 2022

PR description:

It seems we are missing the update on dtSpecsFilter.xml, which happened in 2019, in Phase-2 XMLs (introduced in #26218 and updated in #26535). This leads to the crash of DT superlayer & layer producer in mb3, see report in #36837 (comment).

This code
https://github.com/cms-sw/cmssw/blob/master/Geometry/DTGeometryBuilder/plugins/DTGeometryESModule.cc#L194-L195
looks for MuonBarrelDT in DD4hep detector construction.

I currently migrate only M10. This should be OK if we want to use D88 as a starting point of DD4hep migration. However, migration of M9 or older is fine for me. This is to discuss with Geometry expert and come in the follow up PR.

FYI @slomeo @ianna @civanch @cvuosalo

PR validation:

Local test of workflow 39634.911 with #37005 runs until the ends. We have a warning from HCAL to follow (but it does not stop the job)

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

Not a backport, and no need of backport

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37078/28552

  • This PR adds an extra 28KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @srimanob (Phat Srimanobhas) for master.

It involves the following packages:

  • Configuration/Geometry (geometry, upgrade)
  • Geometry/CMSCommonData (geometry, upgrade)

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

@srimanob
Copy link
Contributor Author

test parameters:

@srimanob
Copy link
Contributor Author

@cmsbuild please test

@srimanob
Copy link
Contributor Author

@cmsbuild please abort

@srimanob
Copy link
Contributor Author

test parameters:

@srimanob
Copy link
Contributor Author

test parameters:

@srimanob
Copy link
Contributor Author

@cmsbuild please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17a74c/22699/summary.html
COMMIT: 31718e3
CMSSW: CMSSW_12_3_X_2022-02-27-0000/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37078/22699/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-17a74c/39434.911_TTbar_14TeV+2026D88_DD4hep+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 4001143
  • DQMHistoTests: Total failures: 8
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 4001113
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 204 log files, 45 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@cvuosalo
Copy link
Contributor

cvuosalo commented Mar 1, 2022

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 1, 2022

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
Copy link
Contributor

perrotta commented Mar 2, 2022

@srimanob do I understand correctly that this PR does not need #37005 for being merged? Should we trigger the tests without that other PR for doing so?

@srimanob
Copy link
Contributor Author

srimanob commented Mar 2, 2022

Hi @perrotta
No, this PR does not need #37005. This PR fix the obsolete xml file defined in Phase2 geometry XML. We can trigger the test as standalone. As far as I understand, the DDD workflow will not affected by the change in this PR. This can be seen in the last test also. Thanks.

@srimanob
Copy link
Contributor Author

srimanob commented Mar 2, 2022

test parameters:

@srimanob
Copy link
Contributor Author

srimanob commented Mar 2, 2022

@cmsbuild please test

Just do the standalone test.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17a74c/22767/summary.html
COMMIT: 31718e3
CMSSW: CMSSW_12_3_X_2022-03-01-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37078/22767/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: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 4000857
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 4000833
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 204 log files, 45 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@srimanob
Copy link
Contributor Author

srimanob commented Mar 2, 2022

I think the test result is as expected. It does not touch DDD workflow.
By the way, do we understand the issue of warning in MTD Failing Comparisons of simPVZ?

@perrotta
Copy link
Contributor

perrotta commented Mar 2, 2022

I think the test result is as expected. It does not touch DDD workflow. By the way, do we understand the issue of warning in MTD Failing Comparisons of simPVZ?

Thank you @srimanob, I'm going to sign this then
About the (long standing) issues of the non reproducible MTD comparisons in DQM, which is totally unrelated from this PR but worth being solved sooner or later, it should be rather addressed to MTD and/or DQM.

@perrotta
Copy link
Contributor

perrotta commented Mar 2, 2022

+1

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.

None yet

4 participants