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 the mismatch in transition ID for idealGeo #39264

Merged
merged 1 commit into from
Sep 1, 2022

Conversation

michael-pitt
Copy link
Contributor

PR description:

This RP fixes an error caused by loading misaligned geometry record together with the aligned geometry record and hit the transitionID error:

The transition ID stored in the ESGetToken does not match the transition where the token is being used

This is happens if a user set buildMisalignedGeometry=True in CTPPSGeometryESModule.cc

PR validation:

cmsrel CMSSW_12_4_3
cd CMSSW_12_4_3/src
cmsenv
voms-proxy-init --voms cms --valid 168:00
cmsRun cfg.py

where the cfg.py is the following:

import FWCore.ParameterSet.Config as cms
 
from Configuration.StandardSequences.Eras import eras
process = cms.Process("trackBasedAlignment", eras.Run3)
 
# minimum of logs
process.MessageLogger = cms.Service("MessageLogger",
  statistics = cms.untracked.vstring(),
  destinations = cms.untracked.vstring('cout'),
  cout = cms.untracked.PSet(
    threshold = cms.untracked.string('WARNING')
  )
)
 
# input data
process.source = cms.Source("PoolSource",
    skipBadFiles = cms.untracked.bool(True),
    fileNames = cms.untracked.vstring(
        '/store/data/Run2022A/ZeroBias/RAW/v1/000/354/329/00000/e2fefed8-a6e8-42d3-90b3-d8df8534769d.root'
    ),
)
 
#load condisions using GT
process.load('Configuration.StandardSequences.FrontierConditions_GlobalTag_cff')
from Configuration.AlCa.GlobalTag import GlobalTag
process.GlobalTag = GlobalTag(process.GlobalTag, '124X_dataRun3_Prompt_Candidate_2022_07_26_15_08_24', '')
 
# PPS reco modules
process.load("EventFilter.CTPPSRawToDigi.ctppsRawToDigi_cff")
process.load("RecoPPS.Configuration.recoCTPPS_cff")
 
# reload geometry
process.load("Geometry.VeryForwardGeometry.geometryRPFromDD_2022_cfi")
process.ctppsGeometryESModule.buildMisalignedGeometry = cms.bool(True) #false will just complain about missing VeryForwardMisalignedGeometryRecord
 
# initial alignments
process.load("CalibPPS.ESProducers.ctppsRPAlignmentCorrectionsDataESSourceXML_cfi")
process.ctppsRPAlignmentCorrectionsDataESSourceXML.RealFiles = cms.vstring()
process.es_prefer_AlignmentCorrections = cms.ESPrefer("CTPPSRPAlignmentCorrectionsDataESSourceXML","ctppsRPAlignmentCorrectionsDataESSourceXML")
 
# aligner
process.load("CalibPPS.AlignmentRelative.ppsStraightTrackAligner_cfi")
 
process.ppsStraightTrackAligner.verbosity = 1
 
process.ppsStraightTrackAligner.tagUVPatternsStrip = cms.InputTag("totemRPUVPatternFinder")
process.ppsStraightTrackAligner.tagDiamondHits = cms.InputTag("")
process.ppsStraightTrackAligner.tagPixelHits = cms.InputTag("")
process.ppsStraightTrackAligner.tagPixelLocalTracks = cms.InputTag("ctppsPixelLocalTracks")
 
process.ppsStraightTrackAligner.maxEvents = int(1E5)
 
process.ppsStraightTrackAligner.rpIds = [103,104,105,123,124,125]
process.ppsStraightTrackAligner.excludePlanes = cms.vuint32()
process.ppsStraightTrackAligner.z0 = +217000
 
process.ppsStraightTrackAligner.maxResidualToSigma = 30
process.ppsStraightTrackAligner.minimumHitsPerProjectionPerRP = 3
 
process.ppsStraightTrackAligner.removeImpossible = True
process.ppsStraightTrackAligner.requireNumberOfUnits = 2
process.ppsStraightTrackAligner.requireOverlap = False
process.ppsStraightTrackAligner.requireAtLeast3PotsInOverlap = True
process.ppsStraightTrackAligner.additionalAcceptedRPSets = ""
 
process.ppsStraightTrackAligner.cutOnChiSqPerNdf = True
process.ppsStraightTrackAligner.chiSqPerNdfCut = 500
 
process.ppsStraightTrackAligner.maxTrackAx = 0.5E-3
process.ppsStraightTrackAligner.maxTrackAy = 0.5E-3
 
optimize="s"
process.ppsStraightTrackAligner.resolveShR = True
process.ppsStraightTrackAligner.resolveShZ = False
process.ppsStraightTrackAligner.resolveRotZ = False
 
process.ppsStraightTrackAligner.constraintsType = "standard"
process.ppsStraightTrackAligner.standardConstraints.units = cms.vuint32(101,121)
process.ppsStraightTrackAligner.oneRotZPerPot = False
process.ppsStraightTrackAligner.useEqualMeanUMeanVRotZConstraints = True
 
process.ppsStraightTrackAligner.algorithms = cms.vstring("Jan")
 
process.ppsStraightTrackAligner.JanAlignmentAlgorithm.stopOnSingularModes = False
 
results_dir="./"
 
process.ppsStraightTrackAligner.taskDataFileName = "" # results_dir + "/task_data.root"
 
process.ppsStraightTrackAligner.fileNamePrefix = results_dir + "/results_iteration_"
process.ppsStraightTrackAligner.expandedFileNamePrefix = results_dir + "/results_cumulative_expanded_"
process.ppsStraightTrackAligner.factoredFileNamePrefix = results_dir + "/results_cumulative_factored_"
 
process.ppsStraightTrackAligner.diagnosticsFile = results_dir + '/diagnostics.root'
process.ppsStraightTrackAligner.buildDiagnosticPlots = True
process.ppsStraightTrackAligner.JanAlignmentAlgorithm.buildDiagnosticPlots = True
 
 
# processing sequence
process.p = cms.Path(
  # it is important to re-run part of the reconstruction as it may influence
  # the choice of rec-hits used in the alignment
  process.ctppsRawToDigi * process.recoCTPPS * process.ppsStraightTrackAligner
)

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39264/31908

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @michael-pitt (Michael Pitt) for master.

It involves the following packages:

  • Geometry/VeryForwardGeometryBuilder (geometry)

@civanch, @Dr15Jones, @makortel, @ianna, @mdhildreth, @cmsbuild, @bsunanda can you please review it and eventually sign? Thanks.
@bsunanda, @fabferro, @fabiocos, @grzanka this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@fabferro
Copy link
Contributor

A new Pull Request was created by @michael-pitt (Michael Pitt) for master.

It involves the following packages:

* Geometry/VeryForwardGeometryBuilder (**geometry**)

@civanch, @Dr15Jones, @makortel, @ianna, @mdhildreth, @cmsbuild, @bsunanda can you please review it and eventually sign? Thanks. @bsunanda, @fabferro, @fabiocos, @grzanka this is something you requested to watch as well. @perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

Since this bug fix is important for PPS alignment data, I would suggest to backport it to 12_4 and 12_5 if still possible.

@civanch
Copy link
Contributor

civanch commented Aug 31, 2022

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d944e6/27240/summary.html
COMMIT: 6364eea
CMSSW: CMSSW_12_6_X_2022-08-31-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39264/27240/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: 51
  • DQMHistoTests: Total histograms compared: 3620448
  • DQMHistoTests: Total failures: 10
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3620416
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 212 log files, 49 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

@civanch
Copy link
Contributor

civanch commented Sep 1, 2022

@michael-pitt , in this PR extra tokens are added:

edm::ESGetToken<DetGeomDesc, IdealGeometryRecord> idealGDToken2_;
edm::ESGetToken<DetGeomDesc, IdealGeometryRecord> idealGDToken_;
edm::ESGetToken<DetGeomDesc, VeryForwardIdealGeometryRecord> idealDBGDToken2_;
edm::ESGetToken<DetGeomDesc, VeryForwardIdealGeometryRecord> idealDBGDToken_;

one can suspect that they cannot be used simultaneously - in the class initialisation there is a choice of one or another. After that the code become complicate, not clear what is initialized what is not.

@michael-pitt
Copy link
Contributor Author

michael-pitt commented Sep 1, 2022

Hi @civanch the extra tokens were added to fix the error. The code was using 3 tokens:

idealGDToken_ = c1.consumesFrom<DetGeomDesc, IdealGeometryRecord>(edm::ESInputTag());
realAlignmentToken_ = c1.consumesFrom<CTPPSRPAlignmentCorrectionsData, RPRealAlignmentRecord>(edm::ESInputTag());
misAlignmentToken_ = c2.consumesFrom<CTPPSRPAlignmentCorrectionsData, RPMisalignedAlignmentRecord>(edm::ESInputTag());

and the error occur when CTPPSGeometryESModule::produceGD consumes idealGDToken_ with misAlignmentToken_ . So I added another consumer

idealGDToken2_ = c2.consumesFrom<DetGeomDesc, IdealGeometryRecord>(edm::ESInputTag());

to avoid mismatch.

btw, in the older version of the code (CMSSW_11_1_X/.../CTPPSGeometryESModule.cc use Tokens, which were initialized with the function. I tried to do the same, but somehow hit multiple errors.

@civanch
Copy link
Contributor

civanch commented Sep 1, 2022

@bsunanda , what do you think? Can we accept this fix?

@makortel
Copy link
Contributor

makortel commented Sep 1, 2022

The replication of tokens is correct. Taking idealGDToken_ as an example, the existing token is registered for use in CTPPSGeometryESModule::produceRealGD()

auto c1 = setWhatProduced(this, &CTPPSGeometryESModule::produceRealGD);
idealGDToken_ = c1.consumesFrom<DetGeomDesc, IdealGeometryRecord>(edm::ESInputTag());

and can not be used in any other produce function. The use in CTPPSGeometryESModule::produceMisalignedGD() needs a separate token object.

@civanch
Copy link
Contributor

civanch commented Sep 1, 2022

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 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, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

perrotta commented Sep 1, 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.

6 participants