[HGC trigger] FE customization, OOT subtraction, separate HFNose sequence #29762

jbsauvan · 2020-05-07T15:43:28Z

PR description:

More customisable frontend selection in the CEE, CEH-si and CEH-sci (@snwebb)
Possibility to configure out-of-time PU subtraction, switched off by default (@jbsauvan)
Separate HFNose and HGCAL in different sequences (@mariadalfonso)
Optimization of pow(2) in floating point compression initialization (@imranyusuff )

PR validation:

Each item in this PR has been validated separately looking at standard distributions. Physics performance studies were also performed when necessary.
Standard tests with V9, V10 and V11 worflows have been run

runTheMatrix.py -w upgrade -l 20034.0 --maxSteps=2
runTheMatrix.py -w upgrade -l 20434.0 --maxSteps=2
runTheMatrix.py -w upgrade -l 22034.0 --maxSteps=2

… CEH and Scintillator

cmsbuild · 2020-05-07T15:43:52Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-05-07T15:49:21Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29762/15209

This PR adds an extra 44KB to repository

cmsbuild · 2020-05-07T15:49:46Z

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

It involves the following packages:

L1Trigger/L1THGCal
L1Trigger/L1THGCalUtilities

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

cms-bot commands are listed here

kpedro88 · 2020-05-07T15:54:23Z

please test

cmsbuild · 2020-05-07T15:54:48Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/6161/console Started: 2020/05/07 17:55

cmsbuild · 2020-05-07T17:10:03Z

+1
Tested at: 8074ea1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6599dd/6161/summary.html
CMSSW: CMSSW_11_1_X_2020-05-07-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-05-07T17:10:06Z

Comparison job queued.

cmsbuild · 2020-05-07T18:38:10Z

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

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: 2697527
DQMHistoTests: Total failures: 54
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2697154
DQMHistoTests: Total skipped: 319
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 · 2020-05-08T11:37:09Z

L1Trigger/L1THGCal/interface/HGCalTriggerTools.h

@@ -85,6 +85,13 @@ class HGCalTriggerTools {

  static constexpr unsigned kScintillatorPseudoThicknessIndex_ = 3;

+  enum SubDetectorType {


This seems redundant with the enums already defined in https://github.com/cms-sw/cmssw/blob/master/DataFormats/ForwardDetId/interface/ForwardSubdetector.h. Is there a reason for it to be separate?

In this case the enum values are used to index vectors. So we need values 0,1,2. The information encoded here is indeed redundant with enums in ForwardSubdetector.h, but we would need to manipulate them further if ForwardSubdetector or HGCalTriggerSubdetector were used.

kpedro88 · 2020-05-08T11:43:19Z

L1Trigger/L1THGCal/plugins/concentrator/HGCalConcentratorProcessorSelection.cc

  }

-  if (coarsenTriggerCells_ || fixedDataSizePerHGCROC_) {
+  for (int subdet = 0; subdet < kNSubDetectors_; subdet++) {
+    if (selectionType[subdet] == "thresholdSelect") {


should these cases become a factory (as some other trigger algorithms are already)?

I am planning to make a global refactoring of the code in the near future. And definitely something should be done to better manage all the different types of algorithms we have in various places.

kpedro88 · 2020-05-08T11:47:45Z

L1Trigger/L1THGCal/python/hgcalBackEndLayer2Producer_cfi.py

+hgcalBackEndLayer2ProducerHFNose = hgcalBackEndLayer2Producer.clone()
+hgcalBackEndLayer2ProducerHFNose.InputCluster = cms.InputTag('hgcalBackEndLayer1ProducerHFNose:HGCalBackendLayer1Processor2DClustering')
+
+binSumsNose = cms.vuint32(13,11,9,9)


probably doesn't need to be a separate variable (just put the values in binSumsHisto a few lines below)

kpedro88 · 2020-05-08T11:49:24Z

L1Trigger/L1THGCal/python/hgcalBackEndLayer2Producer_cfi.py

+
+
+hgcalBackEndLayer2ProducerHFNose = hgcalBackEndLayer2Producer.clone()
+hgcalBackEndLayer2ProducerHFNose.InputCluster = cms.InputTag('hgcalBackEndLayer1ProducerHFNose:HGCalBackendLayer1Processor2DClustering')


actually all of these parameter changes can be done within the clone statement (preferable)

kpedro88 · 2020-05-08T11:54:36Z

L1Trigger/L1THGCal/python/hgcalConcentratorProducer_cfi.py

@@ -160,3 +161,9 @@
    InputTriggerSums = cms.InputTag('hgcalVFEProducer:HGCalVFEProcessorSums'),
    ProcessorParameters = threshold_conc_proc.clone()
    )
+
+
+hgcalConcentratorProducerHFNose = hgcalConcentratorProducer.clone()


make parameter changes inside clone() statement

kpedro88 · 2020-05-08T11:57:05Z

L1Trigger/L1THGCal/src/veryfrontend/HGCalVFELinearizationImpl.cc

-  tdcLSB_sc_ = tdcsaturation_sc_ / pow(2., tdcnBits_sc_);
+      linnBits_(conf.getParameter<uint32_t>("linnBits")),
+      oot_coefficients_(conf.getParameter<std::vector<double>>("oot_coefficients")) {
+  const int kOot_order = 2;


kpedro88 · 2020-05-08T11:58:39Z

L1Trigger/L1THGCal/src/veryfrontend/HGCalVFELinearizationImpl.cc

+      } else {  //ADC mode
+        double data = frame[kIntimeSample].data();
+        // applies OOT PU subtraction only in the ADC mode
+        if (!frame[kIntimeSample - 1].mode()) {


maybe define kSampleN1 and kSampleN2 to reduce repetition?

kpedro88 · 2020-05-08T12:08:26Z

L1Trigger/L1THGCalUtilities/plugins/ntuples/HGCalTriggerNtupleHGCDigis.cc

+    vector<int> hgcdigi_isadc(digi.size());
+    for (int i = 0; i < digi.size(); i++) {
+      hgcdigi_data[i] = digi[i].data();
+      hgcdigi_isadc[i] = (digi[i].mode() ? 0 : 1);


could be just hgcdigi_isadc[i] = !digi[i].mode();

kpedro88 · 2020-05-08T12:08:46Z

L1Trigger/L1THGCalUtilities/plugins/ntuples/HGCalTriggerNtupleHGCDigis.cc

+    vector<int> hgcdigi_isadc(digi.size());
+    for (int i = 0; i < digi.size(); i++) {
+      hgcdigi_data[i] = digi[i].data();
+      hgcdigi_isadc[i] = (digi[i].mode() ? 0 : 1);


could be just hgcdigi_isadc[i] = !digi[i].mode();

kpedro88 · 2020-05-08T12:08:55Z

L1Trigger/L1THGCalUtilities/plugins/ntuples/HGCalTriggerNtupleHGCDigis.cc

+    vector<int> bhdigi_isadc(digi.size());
+    for (int i = 0; i < digi.size(); i++) {
+      bhdigi_data[i] = digi[i].data();
+      bhdigi_isadc[i] = (digi[i].mode() ? 0 : 1);


could be just hgcdigi_isadc[i] = !digi[i].mode();

kpedro88 · 2020-05-12T13:18:50Z

please test

cmsbuild · 2020-05-12T13:19:22Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/6248/console Started: 2020/05/12 15:20

cmsbuild · 2020-05-12T14:36:46Z

+1
Tested at: f63610f
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6599dd/6248/summary.html
CMSSW: CMSSW_11_1_X_2020-05-12-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-05-12T14:36:50Z

Comparison job queued.

cmsbuild · 2020-05-12T16:03:47Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2697527
DQMHistoTests: Total failures: 27
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2697181
DQMHistoTests: Total skipped: 319
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 · 2020-05-12T22:12:32Z

+upgrade

rekovic · 2020-05-17T06:15:47Z

+1

cmsbuild · 2020-05-17T06:16:20Z

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 (and backports should be raised in the release meeting by the corresponding L2)

silviodonato · 2020-05-17T20:44:21Z

+1

snwebb and others added 4 commits May 7, 2020 14:14

Update to allow different concentrator selections in the silicon CEE,…

44feef1

… CEH and Scintillator

Add OOT correction

3798292

separate HFNose and HGCal

50d45df

Optimized power-of-2 FP division

8074ea1

cmsbuild added this to the CMSSW_11_1_X milestone May 7, 2020

cmsbuild added code-checks-pending comparison-pending l1-pending orp-pending pending-signatures tests-pending upgrade-pending labels May 7, 2020

jbsauvan mentioned this pull request May 7, 2020

List of integrations PFCal-dev/cmssw#44

Open

cmsbuild added code-checks-approved and removed code-checks-pending labels May 7, 2020

cmsbuild added tests-started and removed tests-pending labels May 7, 2020

cmsbuild added tests-approved and removed tests-started labels May 7, 2020

cmsbuild added comparison-available and removed comparison-pending labels May 7, 2020

kpedro88 reviewed May 8, 2020

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels May 12, 2020

cmsbuild added tests-approved and removed tests-started labels May 12, 2020

cmsbuild added comparison-available and removed comparison-pending labels May 12, 2020

cmsbuild added upgrade-approved and removed upgrade-pending labels May 12, 2020

cmsbuild added fully-signed l1-approved and removed l1-pending pending-signatures labels May 17, 2020

cmsbuild added orp-approved and removed orp-pending labels May 17, 2020

cmsbuild merged commit 1f30a93 into cms-sw:master May 17, 2020

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

Comments cms-sw#29762

6a17233

jbsauvan mentioned this pull request May 19, 2020

Backport comments PR 29762 PFCal-dev/cmssw#427

Merged

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

Comments cms-sw#29762

7a5bcbc

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

Comments cms-sw#29762

fcf7d5a

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

Comments cms-sw#29762

7933e99

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

[HGC trigger] FE customization, OOT subtraction, separate HFNose sequence #29762

[HGC trigger] FE customization, OOT subtraction, separate HFNose sequence #29762

jbsauvan commented May 7, 2020

cmsbuild commented May 7, 2020

cmsbuild commented May 7, 2020

cmsbuild commented May 7, 2020

kpedro88 commented May 7, 2020

cmsbuild commented May 7, 2020 •

edited

cmsbuild commented May 7, 2020

cmsbuild commented May 7, 2020

cmsbuild commented May 7, 2020

kpedro88 May 8, 2020

jbsauvan May 10, 2020

kpedro88 May 8, 2020

jbsauvan May 10, 2020

kpedro88 May 8, 2020

kpedro88 May 8, 2020

kpedro88 May 8, 2020

kpedro88 May 8, 2020

kpedro88 May 8, 2020

kpedro88 May 8, 2020

kpedro88 May 8, 2020

kpedro88 May 8, 2020

kpedro88 commented May 12, 2020

cmsbuild commented May 12, 2020 •

edited

cmsbuild commented May 12, 2020

cmsbuild commented May 12, 2020

cmsbuild commented May 12, 2020

kpedro88 commented May 12, 2020

rekovic commented May 17, 2020

cmsbuild commented May 17, 2020

silviodonato commented May 17, 2020

		@@ -85,6 +85,13 @@ class HGCalTriggerTools {

		static constexpr unsigned kScintillatorPseudoThicknessIndex_ = 3;

		enum SubDetectorType {



		hgcalBackEndLayer2ProducerHFNose = hgcalBackEndLayer2Producer.clone()
		hgcalBackEndLayer2ProducerHFNose.InputCluster = cms.InputTag('hgcalBackEndLayer1ProducerHFNose:HGCalBackendLayer1Processor2DClustering')

[HGC trigger] FE customization, OOT subtraction, separate HFNose sequence #29762

[HGC trigger] FE customization, OOT subtraction, separate HFNose sequence #29762

Conversation

jbsauvan commented May 7, 2020

PR description:

PR validation:

cmsbuild commented May 7, 2020

cmsbuild commented May 7, 2020

cmsbuild commented May 7, 2020

kpedro88 commented May 7, 2020

cmsbuild commented May 7, 2020 • edited

cmsbuild commented May 7, 2020

cmsbuild commented May 7, 2020

cmsbuild commented May 7, 2020

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

kpedro88 commented May 12, 2020

cmsbuild commented May 12, 2020 • edited

cmsbuild commented May 12, 2020

cmsbuild commented May 12, 2020

cmsbuild commented May 12, 2020

kpedro88 commented May 12, 2020

rekovic commented May 17, 2020

cmsbuild commented May 17, 2020

silviodonato commented May 17, 2020

cmsbuild commented May 7, 2020 •

edited

cmsbuild commented May 12, 2020 •

edited