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
[HGCal trigger] Remove deprecated v8 geometry code + Add new trigger geometry class #34608
Conversation
One line bug-fix for getOrderedTriggerCellsFromModule
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34608/24174
|
A new Pull Request was created by @jbsauvan (Jean-Baptiste Sauvan) for master. It involves the following packages:
@civanch, @mdhildreth, @cmsbuild, @rekovic, @srimanob, @kpedro88, @cecilecaillol can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
test parameters: |
please test with cms-data/L1Trigger-L1THGCal#22 |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1a3835/17194/summary.html CMS StaticAnalyzer warnings: There are 4 EventSetupRecord::get warnings. See https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1a3835/17194/llvm-analysis/esrget-sa.txt for details. Comparison SummarySummary:
|
+1 |
+l1 |
totalLayers_ = eeLayers_ + fhLayers_ + bhLayers_; | ||
} | ||
bhLayers_ = geom_->hscTopology().dddConstants().layers(true); | ||
totalLayers_ = eeLayers_ + fhLayers_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the definition of totalLayers_ changed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The definition of totalLayers_
as totalLayers_ = eeLayers_ + fhLayers_ + bhLayers_
was only for geometries <=V8, where the HCAL layers were divided into front and back sections. In geometries >= V9, the silicon section of HCAL is covering all the HCAL layers (given by fhLayers_
).
Before there was a switch to check whether the geometry was <=V8 or >=V9; now the switch has been removed. So the definition of totalLayers_
for geometries >=V9 has not changed.
Just wonder, in addition of the PR, do you still want to keep
|
The method |
+Upgrade |
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. @silviodonato, @dpiparo, @qliphy, @perrotta (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
HGCalTriggerGeometryV9Imp3
containing information on the connections between modules, lpGBT links and backend FPGAs, read from a json file.Currently not used by default, but will replace
HGCalTriggerGeometryV9Imp2
in the future.Associated internal PRs and reviews:
This PR depends on external cms-data/L1Trigger-L1THGCal#22
PR validation:
Private tests in
L1Trigger/L1THGCalUtilities/test
:Tested D49, D60 and D68 workflows
@snwebb FYI