[HGCAL trigger] Integrate HFNose #28461

jbsauvan · 2019-11-24T09:57:43Z

PR description:

Integrate the HFNose in the HGCAL trigger primitive generation.

PR validation:

Standard code checks and formatting.

scram b code-checks
scram b code-format

Test C9, C9, C10 workflows.

runTheMatrix.py -w upgrade -l 21634.0 --maxSteps=2
runTheMatrix.py -w upgrade -l 22034.0 --maxSteps=2
runTheMatrix.py -w upgrade -l 22434.0 --maxSteps=2

cmsbuild · 2019-11-24T09:58:13Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-11-24T10:03:24Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28461/12895

This PR adds an extra 80KB to repository

cmsbuild · 2019-11-24T10:03:57Z

A new Pull Request was created by @jbsauvan (Jean-Baptiste Sauvan) for master.

It involves the following packages:

DataFormats/L1THGCal
Fireworks/Calo
L1Trigger/L1THGCal

@benkrikler, @Dr15Jones, @alja, @kpedro88, @cmsbuild, @rekovic can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @alja, @amarini, @rovere, @lgray this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

fabiocos · 2019-11-24T17:00:25Z

please test workflow 23234.0

cmsbuild · 2019-11-24T17:02:45Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/3607/console Started: 2019/11/24 18:04

cmsbuild · 2019-11-24T19:00:23Z

+1
Tested at: d00974e
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f9bcc8/3607/summary.html

cmsbuild · 2019-11-24T19:00:27Z

Comparison job queued.

cmsbuild · 2019-11-24T20:11:28Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f9bcc8/3607/summary.html

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

/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-f9bcc8/23234.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2026D49_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D49+RecoFullGlobal_2026D49+HARVESTFullGlobal_2026D49

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 3 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2785613
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2785270
DQMHistoTests: Total skipped: 341
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 147 log files, 16 edm output root files, 34 DQM output files

kpedro88 · 2019-11-25T16:41:50Z

L1Trigger/L1THGCal/plugins/geometries/HGCalTriggerGeometryHexImp1.cc

@@ -223,7 +235,7 @@ void HGCalTriggerGeometryHexImp1::buildTriggerCellsAndModules()
    unsigned triggerCell = cell_triggerCell.second;
    auto itr_exist = trigger_cells_to_cells.emplace(triggerCell, list_cells());
    itr_exist.first->second.emplace(cell);
-    //trigger_cells_to_cells.at(triggerCell).emplace(cell);
+    // trigger_cells_to_cells.at(triggerCell).emplace(cell);


delete commented-out code (everywhere in this file)

kpedro88 · 2019-11-25T16:42:19Z

L1Trigger/L1THGCal/plugins/geometries/HGCalTriggerGeometryImp1.cc

@@ -116,7 +128,7 @@ void HGCalTriggerGeometryImp1::buildMaps()
    unsigned moduleId =
        (tc2mItr != trigger_cells_to_modules_.end() ? tc2mItr->second
                                                    : 0);  // 0 if the trigger cell doesn't belong to a module
-    //unsigned moduleId = trigger_cells_to_modules_.at(triggercellId);
+    // unsigned moduleId = trigger_cells_to_modules_.at(triggercellId);


delete commented-out code

kpedro88 · 2019-11-25T16:47:05Z

L1Trigger/L1THGCal/python/hgcalBackEndLayer2Producer_cfi.py

-                               )
+                              )
+
+## Note: this customization change both HGC and HFnose, to be revised


should this be addressed within this PR?

I have discussed with @jbsauvan
The plan is to duplicate the full L1 sequence instead of having one single object with both HGC and HFnose TP.

I intended to do in subsequent PR ; is it ok ?

The duplication of the sequence for the HFNose might not be straightforward. In any case it will require checks and validation. So I would prefer to keep this for an other PR.

kpedro88 · 2019-11-25T16:47:15Z

L1Trigger/L1THGCal/python/hgcalVFEProducer_cfi.py

+# NOSE parameters
+fCPerMIPnose = recoparam.HGCalUncalibRecHit.HGCHFNoseConfig.fCPerMIP
+layerWeightsNose = recocalibparam.dEdX.weightsNose
+##thicknessCorrectionNose = recocalibparam.HGCalRecHit.thicknessNoseCorrection


delete commented-out code

Hardcoded lines 37
thicknessCorrectionNose = [0.759,0.760,0.773]
should be removed and line 36 uncommented

kpedro88 · 2019-11-25T16:47:35Z

L1Trigger/L1THGCal/src/HGCalTriggerTools.cc

@@ -53,6 +58,8 @@ GlobalPoint HGCalTriggerTools::getTCPosition(const DetId& id) const {
    // to distinguish a TC detId
  }

+  // FIXME: to do HFNose


should this be fixed in this PR?

In principle it is ok, since it delegates the calculation to the geometry which correctly computes the nose TC positions.
comment can be removed

kpedro88 · 2019-11-25T16:47:49Z

L1Trigger/L1THGCal/src/HGCalTriggerTools.cc

@@ -90,6 +100,7 @@ unsigned HGCalTriggerTools::layers(DetId::Detector type) const {
    case DetId::HGCalHSc:
      layers = bhLayers_;
      break;
+    // FIXME: to do HFNose


should this be fixed in this PR?

@mariadalfonso @jbsauvan please check this

In HGCalTriggerTools::layers(DetId::Detector type) I do not have access to the subdet so cannot set up the switch to the HFNose.

Downstream the code the HGCalTriggerTools::layers(ForwardSubdetector type) is in use.
See for example the changes in https://github.com/cms-sw/cmssw/pull/28461/files#diff-ffd813bde161fec49fe1259f558ea728R91

This function cannot handle HFNose.
At some point we should think if there is a better way to handle all cases, maybe merging the different layers functions. But it will require to check everywhere the functions are called. I propose to do this in an other PR. We can keep the FIXME in the code to remember that.

kpedro88 · 2019-11-25T16:49:29Z

L1Trigger/L1THGCal/test/testHGCalL1TGeometryV9Imp2_Nose_cfg.py

+import FWCore.ParameterSet.Config as cms
+
+
+from Configuration.Eras.Era_Phase2C6_timing_layer_bar_cff import Phase2C6_timing_layer_bar


this should be updated with respect to #28387

alja · 2019-11-25T18:30:27Z

+1

cmsbuild · 2019-12-02T18:10:51Z

+1
Tested at: 6d0b206
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f9bcc8/3725/summary.html

cmsbuild · 2019-12-02T18:10:54Z

Comparison job queued.

cmsbuild · 2019-12-02T19:22:22Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f9bcc8/3725/summary.html

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

/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-f9bcc8/23234.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2026D49_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D49+RecoFullGlobal_2026D49+HARVESTFullGlobal_2026D49

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2793840
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2793498
DQMHistoTests: Total skipped: 341
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 147 log files, 16 edm output root files, 34 DQM output files

kpedro88 · 2019-12-03T16:38:10Z

+upgrade
outstanding comments to be addressed in followup PR:
#28461 (comment)
#28461 (comment)

fabiocos · 2019-12-05T15:38:52Z

@alja the changes in the visualisation area look technical and minor, could you please have a look?

@rekovic this PR needs your attention

alja · 2019-12-05T18:44:28Z

+1

rekovic · 2019-12-09T04:42:02Z

+1

cmsbuild · 2019-12-09T04:42:32Z

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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

fabiocos · 2019-12-09T09:36:47Z

+1

add L1 geometry and initial trigger cells

d00974e

cmsbuild added this to the CMSSW_11_0_X milestone Nov 24, 2019

cmsbuild added code-checks-pending comparison-pending l1-pending orp-pending pending-signatures tests-pending upgrade-pending visualization-pending labels Nov 24, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 24, 2019

cmsbuild added tests-started and removed tests-pending labels Nov 24, 2019

cmsbuild added tests-approved and removed tests-started labels Nov 24, 2019

cmsbuild added comparison-available and removed comparison-pending labels Nov 24, 2019

jbsauvan mentioned this pull request Nov 25, 2019

List of integrations PFCal-dev/cmssw#44

Open

kpedro88 reviewed Nov 25, 2019

View reviewed changes

cmsbuild removed the visualization-pending label Nov 25, 2019

cmsbuild added tests-started and removed tests-pending labels Dec 2, 2019

cmsbuild added tests-approved and removed tests-started labels Dec 2, 2019

cmsbuild added comparison-available and removed comparison-pending labels Dec 2, 2019

cmsbuild added upgrade-approved and removed upgrade-pending labels Dec 3, 2019

cmsbuild mentioned this pull request Dec 5, 2019

PFlow fix/improvement in the HF region for phase2 #28442

Merged

cmsbuild added visualization-approved and removed visualization-pending labels Dec 5, 2019

cmsbuild mentioned this pull request Dec 6, 2019

[HGCAL trigger] Fix percentile out of bound #28570

Merged

cmsbuild added fully-signed l1-approved and removed l1-pending pending-signatures labels Dec 9, 2019

cmsbuild added orp-approved and removed orp-pending labels Dec 9, 2019

cmsbuild merged commit b6a7209 into cms-sw:master Dec 9, 2019

jbsauvan added a commit to PFCal-dev/cmssw that referenced this pull request Apr 7, 2020

Implement comments cms-sw#28461

c0f2cfc

jbsauvan mentioned this pull request Nov 29, 2021

Archived Integrations hgc-tpg/cmssw#1

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HGCAL trigger] Integrate HFNose #28461

[HGCAL trigger] Integrate HFNose #28461

jbsauvan commented Nov 24, 2019

cmsbuild commented Nov 24, 2019

cmsbuild commented Nov 24, 2019

cmsbuild commented Nov 24, 2019

fabiocos commented Nov 24, 2019

cmsbuild commented Nov 24, 2019 •

edited

Loading

cmsbuild commented Nov 24, 2019

cmsbuild commented Nov 24, 2019

cmsbuild commented Nov 24, 2019

kpedro88 Nov 25, 2019

kpedro88 Nov 25, 2019

kpedro88 Nov 25, 2019

mariadalfonso Nov 25, 2019

jbsauvan Nov 26, 2019

kpedro88 Nov 25, 2019

mariadalfonso Nov 25, 2019

kpedro88 Nov 25, 2019

mariadalfonso Nov 25, 2019

kpedro88 Nov 25, 2019

kpedro88 Nov 26, 2019

mariadalfonso Nov 26, 2019

jbsauvan Nov 27, 2019

kpedro88 Nov 25, 2019

alja commented Nov 25, 2019

cmsbuild commented Dec 2, 2019

cmsbuild commented Dec 2, 2019

cmsbuild commented Dec 2, 2019

kpedro88 commented Dec 3, 2019

fabiocos commented Dec 5, 2019

alja commented Dec 5, 2019

rekovic commented Dec 9, 2019

cmsbuild commented Dec 9, 2019

fabiocos commented Dec 9, 2019

		import FWCore.ParameterSet.Config as cms


		from Configuration.Eras.Era_Phase2C6_timing_layer_bar_cff import Phase2C6_timing_layer_bar

[HGCAL trigger] Integrate HFNose #28461

[HGCAL trigger] Integrate HFNose #28461

Conversation

jbsauvan commented Nov 24, 2019

PR description:

PR validation:

cmsbuild commented Nov 24, 2019

cmsbuild commented Nov 24, 2019

cmsbuild commented Nov 24, 2019

fabiocos commented Nov 24, 2019

cmsbuild commented Nov 24, 2019 • edited Loading

cmsbuild commented Nov 24, 2019

cmsbuild commented Nov 24, 2019

cmsbuild commented Nov 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alja commented Nov 25, 2019

cmsbuild commented Dec 2, 2019

cmsbuild commented Dec 2, 2019

cmsbuild commented Dec 2, 2019

kpedro88 commented Dec 3, 2019

fabiocos commented Dec 5, 2019

alja commented Dec 5, 2019

rekovic commented Dec 9, 2019

cmsbuild commented Dec 9, 2019

fabiocos commented Dec 9, 2019

cmsbuild commented Nov 24, 2019 •

edited

Loading