[HGCAL trigger] Fix percentile out of bound

[jbsauvan]

PR description:

Fix out of bound in layer percentile interpolation. See issue #28566.

PR validation:

Ran on HGCAL V9 sample.
Tested extreme case where no trigger cell are selected in the computation, which gives 0 energy in all the layers.

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28570/13062

• This PR adds an extra 16KB to repository

• There are other open Pull requests which might conflict with changes you have proposed:

  • File L1Trigger/L1THGCal/src/backend/HGCalShowerShape.cc modified in PR(s): [HGCAL trigger] Integrate HFNose #28461

[cmsbuild]

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

It involves the following packages:

L1Trigger/L1THGCal

@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.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[kpedro88]

please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/3849/console Started: 2019/12/06 16:51

[Dr15Jones]

Can layers.size() == 0? If so then calling layers.back() in that case reads off the end (and may cause a segmentation fault since it is likely to be reading memory location 0).

[jbsauvan]

The size of layers is given by the number of HGCAL layers :

cmssw/L1Trigger/L1THGCal/src/backend/HGCalShowerShape.cc

Lines 111 to 112 in 04f9376

	unsigned nlayers = triggerTools_.layers(ForwardSubdetector::ForwardEmpty);
	std::vector<double> layers(nlayers, 0);

where the trigger tools returns this:

cmssw/L1Trigger/L1THGCal/src/HGCalTriggerTools.cc

Lines 72 to 73 in 04f9376

	case ForwardSubdetector::ForwardEmpty:
	layers = totalLayers_;

Which is computed from the HGCAL geometry:

cmssw/L1Trigger/L1THGCal/src/HGCalTriggerTools.cc

Lines 38 to 47 in 04f9376

	eeLayers_ = geom_->eeTopology().dddConstants().layers(true);
	fhLayers_ = geom_->fhTopology().dddConstants().layers(true);
	if (geom_->isV9Geometry()) {
	bhLayers_ = geom_->hscTopology().dddConstants().layers(true);
	totalLayers_ = eeLayers_ + fhLayers_;
	} else {
	bhLayers_ = geom_->bhTopology().dddConstants()->getMaxDepth(1);
	totalLayers_ = eeLayers_ + fhLayers_ + bhLayers_;
	}
	}

So unless something really bad happens it shouldn't be 0

[cmsbuild]

+1
Tested at: b86e9b9
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1650e4/3849/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1650e4/3849/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: 2800084
• DQMHistoTests: Total failures: 1
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2799742
• 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

[fabiocos]

@kpedro88 @rekovic please have a look, this looks a technical fix which should be ok (unless there is a serious inconsistency in the geometry definition).

[kpedro88]

+upgrade

[fabiocos]

+1

@rekovic please have a look

[fabiocos]

merge