Skip to content

Conversation

@bsunanda
Copy link
Contributor

PR description:

Add the tools with some more diagnostic help to debug the issue of wrong DetID->Position->DetID assignment

PR validation:

Tested for the diagnostic

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Nothing special

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 11, 2026

cms-bot internal usage

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49763/47353

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @bsunanda for master.

It involves the following packages:

  • Geometry/HGCalCommonData (geometry)

@Dr15Jones, @bsunanda, @civanch, @cmsbuild, @kpedro88, @makortel, @mdhildreth can you please review it and eventually sign? Thanks.
@fabiocos, @martinamalberti this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@bsunanda
Copy link
Contributor Author

@cmsbuild Please test

@bsunanda
Copy link
Contributor Author

@cmsbuild Please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals
Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d20885/50505/summary.html
COMMIT: f31de82
CMSSW: CMSSW_16_1_X_2026-01-11-0000/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/49763/50505/install.sh to create a dev area with all the needed externals and cmssw changes.

Failed RelVals

----- Begin Fatal Exception 11-Jan-2026 16:26:31 CET-----------------------
An exception of category 'OutOfBound' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 4 stream: 0
   [1] Running path 'HLTriggerFinalPath'
   [2] Prefetching for module TriggerSummaryProducerAOD/'hltTriggerSummaryAOD'
   [3] Prefetching for module L1HPSPFTauProducer/'l1tHPSPFTauProducer'
   [4] Prefetching for module L1TPFCandMultiMerger/'l1tLayer1'
   [5] Prefetching for module L1TCorrelatorLayer1Producer/'l1tLayer1HGCal'
   [6] Calling method for module HGCalBackendLayer2Producer/'l1tHGCalBackEndLayer2Producer'
Exception Message:
TC X1 = 0.0713466 out of the seeding histogram bounds 0.076 - 0.58
----- End Fatal Exception -------------------------------------------------

@bsunanda
Copy link
Contributor Author

This error is not due to the changes of the PR. It needs investigation

@bsunanda
Copy link
Contributor Author

+geometry

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @mandrenguyen, @ftenchini, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2)

@mandrenguyen
Copy link
Contributor

The exception is coming from this line:

throw cms::Exception("OutOfBound") << "TC X1 = " << x1 << " out of the seeding histogram bounds " << minx1

It's not clear to me why this is only popping up in this PR and not others. Would you expect a change in geometry could cause us to hit the error by accident?

@bsunanda
Copy link
Contributor Author

bsunanda commented Jan 12, 2026 via email

@makortel
Copy link
Contributor

The exception is coming from this line:

throw cms::Exception("OutOfBound") << "TC X1 = " << x1 << " out of the seeding histogram bounds " << minx1

It's not clear to me why this is only popping up in this PR and not others. Would you expect a change in geometry could cause us to hit the error by accident?

Is #41451 back?

@bsunanda
Copy link
Contributor Author

bsunanda commented Jan 12, 2026 via email

@makortel
Copy link
Contributor

Is the pileup file made with the same CMSSW version - I mean the IB + current PR?

The premixed pileup file itself is done with the same CMSSW version, but MinBias files used as an input for the premixing are read from storage (I see from CMSSW_16_0_0_pre2). If that mismatch is the problem, then we just have to accept the failures until a new MinBias dataset is produced.

(it would be great if someone would have time to craft another variant of the workflow that would generate the MinBias on the fly)

@kpedro88
Copy link
Contributor

(it would be great if someone would have time to craft another variant of the workflow that would generate the MinBias on the fly)

Actually, these exist for alternative pileup options (premixing and hybrid), just not for classical mixing... @makortel could you open an issue for this?

@makortel
Copy link
Contributor

makortel commented Jan 12, 2026

(it would be great if someone would have time to craft another variant of the workflow that would generate the MinBias on the fly)

Actually, these exist for alternative pileup options (premixing and hybrid), just not for classical mixing... @makortel could you open an issue for this?

I can open an issue, but just to clarify, I was talking about premixing. While the X.999 does the premixing on the fly, it relies on existing MinBias files.

@makortel
Copy link
Contributor

(it would be great if someone would have time to craft another variant of the workflow that would generate the MinBias on the fly)

Actually, these exist for alternative pileup options (premixing and hybrid), just not for classical mixing... @makortel could you open an issue for this?

I can open an issue, but just to clarify, I was talking about premixing. While the X.999 does the premixing on the fly, it relies on existing MinBias files.

I opened #49776 for the case I had in mind. I can extend the issue to classical mixing if you want (but maybe we should move that discussion to the issue)

@kpedro88
Copy link
Contributor

@makortel thanks for clarifying. Yes, I would include both in the same issue.

@mandrenguyen
Copy link
Contributor

+1
As discussed above the failure stems from the input file and gets triggered by the change in geometry. has nothing to do with this PR.

@mandrenguyen
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 2cb07a0 into cms-sw:master Jan 12, 2026
9 of 10 checks passed
@akritkbehera
Copy link
Contributor

Some compilation errors were found in CMSSW_16_1_UBSAN_X IBs after this PR
constexpr std::string compilation error in HGCalTypes.h

static constexpr std::string waferType_[4] = {"HD120", "LD200", "LD300", "HD200"};
static constexpr std::string waferTypeX_[27] = {
"Full", "Five", "ChopTwo", "ChopTwoM", "Half", "Semi", "Semi2", "Three", "Half2",
"Five2", "Unknown10", "LDTop", "LDBottom", "LDLeft", "LDRight", "LDFive", "LDThree", "Unknown17",
"Unknown18", "Unknown19", "Unknown20", "HDTop", "HDBottom", "HDLeft", "HDRight", "HDFive", "Out"};
};

In file included from src/Geometry/HGCalCommonData/interface/HGCalDDDConstants.h:26,
                 from src/CondFormats/HGCalObjects/interface/HeterogeneousHGCalHEFCellPositionsConditions.h:11,
                 from src/CondFormats/HGCalObjects/src/T_EventSetup_HeterogeneousHGCalHEFCellPositionsConditions.cc:1:
  src/Geometry/HGCalCommonData/interface/HGCalTypes.h:167:85: error: '(((const std::string*)(& HGCalTypes::layerTypes_)) != 0)' is not a constant expression
   167 |       "Center", "CenterB", "CenterYp", "CenterYm", "CenterR", "CenterXp", "CenterXm"};
      |                                                                                     ^
  src/Geometry/HGCalCommonData/interface/HGCalTypes.h:168:83: error: '(((const std::string*)(& HGCalTypes::waferType_)) != 0)' is not a constant expression
   168 |   static constexpr std::string waferType_[4] = {"HD120", "LD200", "LD300", "HD200"};
      |                                                                                   ^
  src/Geometry/HGCalCommonData/interface/HGCalTypes.h:172:108: error: '(((const std::string*)(& HGCalTypes::waferTypeX_)) != 0)' is not a constant expression
   172 |       "Unknown18", "Unknown19", "Unknown20", "HDTop",    "HDBottom", "HDLeft",  "HDRight", "HDFive",  "Out"};

@mmusich
Copy link
Contributor

mmusich commented Jan 15, 2026

As discussed above the failure stems from the input file and gets triggered by the change in geometry. has nothing to do with this PR.

On the contrary, I think this PR is the primary cause of the recent turbulence in IBs.
For example in the test revert #49831, the GPU matrix passed mostly unscathed (while it fails elsewhere).
Can we consider a revert?
@bsunanda @mandrenguyen

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.

7 participants