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 ge2/1 demonstrator in validation #36835

Merged
merged 7 commits into from Feb 2, 2022

Conversation

watson-ij
Copy link
Contributor

PR description:

Fix issues in #36826 caused by a missing the demonstrator superchamber in the db geometry. Also fix an issue in Validation/MuonGEMHits which might not build station 2 when it exists for the demonstrator.

PR validation:

Ran the PSet referenced in the issue, checked it ran the crashing event.

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:

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36835/28018

  • 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

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36835/28019

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @watson-ij (Ian J. Watson) for master.

It involves the following packages:

  • Geometry/GEMGeometryBuilder (geometry, upgrade)
  • Validation/MuonGEMHits (dqm)

@civanch, @Dr15Jones, @makortel, @cvuosalo, @emanueleusai, @ianna, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob, @jfernan2, @ahmad3213, @pmandrik, @pbo0, @rvenditti can you please review it and eventually sign? Thanks.
@jshlee, @bsunanda, @fabiocos, @slomeo, @giovanni-mocellin 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

@watson-ij
Copy link
Contributor Author

please test

@qliphy
Copy link
Contributor

qliphy commented Jan 30, 2022

type bugfix

@perrotta
Copy link
Contributor

urgent

@srimanob
Copy link
Contributor

Do we need to run .911 workflow to make sure that we don't have an issue with DD4hep XML?

@cmsbuild
Copy link
Contributor

-1

Failed Tests: ClangBuild
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25a387/22081/summary.html
COMMIT: 9860b88
CMSSW: CMSSW_12_3_X_2022-01-29-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36835/22081/install.sh to create a dev area with all the needed externals and cmssw changes.

Clang Build

I found compilation warning while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

See details on the summary page.

@cvuosalo
Copy link
Contributor

+1

@cvuosalo
Copy link
Contributor

I created and uploaded updated GEM reco geometry with this PR, and it is in the DB with tag GEMRECO_Geometry_123DD4hepV2. See my comment in #36826 for more details:
#36826 (comment)

@cvuosalo
Copy link
Contributor

cvuosalo commented Jan 31, 2022

@watson-ij Can you validate GEM reco geometry? I can tell that the update GEM reco geometry made with this PR is different from the previous version, and I can print out the contents the GEM reco geometry, but I don't know how to verify its correctness.
The candidate GT with the updated reco geometry from this PR is 123X_mcRun3_2021_design_Candidate_2022_01_31_23_02_13.

@watson-ij
Copy link
Contributor Author

Thank you @cvuosalo, if I use the new tag [1] with the PSet that was crashing, I see the demo. superchamber, and so station 2 is built, and the event which was crashing is successfully processed without issue.

[1] To the PSet I was testing, I just added the lines:

process.GlobalTag.toGet = cms.VPSet(
        cms.PSet(record = cms.string("GEMRecoGeometryRcd"),
                 tag = cms.string("GEMRECO_Geometry_123DD4hepV2"),
                 connect = cms.string("frontier://FrontierProd/CMS_CONDITIONS"),
                 )
        )

@srimanob
Copy link
Contributor

srimanob commented Feb 1, 2022

+Upgrade

Test by running 1k events of ttbar and ZMM from scratch, jobs run fine. Detail is in
#36826 (comment)

@jfernan2
Copy link
Contributor

jfernan2 commented Feb 1, 2022

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 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)

@cvuosalo
Copy link
Contributor

cvuosalo commented Feb 1, 2022

For DDD, when the GEM reco geometry payload is being created, this PR causes a segmentation violation. DD4hep is OK. I've confirmed the crash, and now I am trying to figure out which lines of code to change to fix it.

@cvuosalo
Copy link
Contributor

cvuosalo commented Feb 2, 2022

I have a fix that allows the DDD GEM reco geometry payload to be created. Maybe this PR should be merged, and then I will submit the fix in a new PR. The problem does not affect any workflow, so it is safe to merge this PR.

@perrotta
Copy link
Contributor

perrotta commented Feb 2, 2022

+1

@cmsbuild cmsbuild merged commit fddd406 into cms-sw:master Feb 2, 2022
@srimanob
Copy link
Contributor

srimanob commented Feb 2, 2022

Thanks @cvuosalo

@cvuosalo
Copy link
Contributor

cvuosalo commented Feb 2, 2022

PR #36869 provides a fix for creating the DDD GEM reco geometry DB payload.

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

8 participants