Skip to content

Commit

Permalink
Merge pull request #38148 from missirol/devel_fixZeroADCOfPixelDigi_124X
Browse files Browse the repository at this point in the history 
guard against `uint16_t` overflows of Pixel ADC values [`12_4_X`]
  • Loading branch information
@cmsbuild
cmsbuild committed Jun 1, 2022
2 parents a189a1d + 8332d36 commit 3e286ef
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 28 deletions.
27 changes: 16 additions & 11 deletions RecoLocalTracker/SiPixelClusterizer/plugins/PixelThresholdClusterizer.cc
View file
Expand Up @@ -33,7 +33,8 @@
#include <vector>
#include <iostream>
#include <atomic>
using namespace std;
#include <algorithm>
#include <limits>

//----------------------------------------------------------------------------
//! Constructor:
Expand Down Expand Up @@ -408,8 +409,8 @@ int PixelThresholdClusterizer::calibrate(int adc, int col, int row) {
SiPixelCluster PixelThresholdClusterizer::make_cluster(const SiPixelCluster::PixelPos& pix,
edmNew::DetSetVector<SiPixelCluster>::FastFiller& output) {
//First we acquire the seeds for the clusters
int seed_adc;
stack<SiPixelCluster::PixelPos, vector<SiPixelCluster::PixelPos> > dead_pixel_stack;
uint16_t seed_adc;
std::stack<SiPixelCluster::PixelPos, std::vector<SiPixelCluster::PixelPos> > dead_pixel_stack;

//The individual modules have been loaded into a buffer.
//After each pixel has been considered by the clusterizer, we set the adc count to 1
Expand All @@ -428,7 +429,10 @@ SiPixelCluster PixelThresholdClusterizer::make_cluster(const SiPixelCluster::Pix
}
else {
*/
seed_adc = theBuffer(pix.row(), pix.col());
// Note: each ADC value is limited here to 65535 (std::numeric_limits<uint16_t>::max),
// as it is later stored as uint16_t in SiPixelCluster and PixelClusterizerBase/AccretionCluster
// (reminder: ADC values here may be expressed in number of electrons)
seed_adc = std::min(theBuffer(pix.row(), pix.col()), int(std::numeric_limits<uint16_t>::max()));
theBuffer.set_adc(pix, 1);
// }

Expand All @@ -450,11 +454,12 @@ SiPixelCluster PixelThresholdClusterizer::make_cluster(const SiPixelCluster::Pix
++r) {
if (theBuffer(r, c) >= thePixelThreshold) {
SiPixelCluster::PixelPos newpix(r, c);
if (!acluster.add(newpix, theBuffer(r, c)))
auto const newpix_adc = std::min(theBuffer(r, c), int(std::numeric_limits<uint16_t>::max()));
if (!acluster.add(newpix, newpix_adc))
goto endClus;
// VV: no fake pixels in cluster, leads to non-contiguous clusters
if (!theFakePixels[r * theNumOfCols + c]) {
cldata.add(newpix, theBuffer(r, c));
cldata.add(newpix, newpix_adc);
}
theBuffer.set_adc(newpix, 1);
}
Expand All @@ -471,8 +476,8 @@ SiPixelCluster PixelThresholdClusterizer::make_cluster(const SiPixelCluster::Pix
SiPixelCluster::PixelPos newpix(r,c);
if(!doSplitClusters){
cluster.add(newpix, theBuffer(r,c));}
cluster.add(newpix, std::min(theBuffer(r, c), int(std::numeric_limits<uint16_t>::max())));}
else if(doSplitClusters){
dead_pixel_stack.push(newpix);
dead_flag = true;}
Expand Down Expand Up @@ -518,9 +523,9 @@ SiPixelCluster PixelThresholdClusterizer::make_cluster(const SiPixelCluster::Pix
//This loop adds the second cluster to the first.
const std::vector<SiPixelCluster::Pixel>& branch_pixels = second_cluster.pixels();
for (unsigned int i = 0; i < branch_pixels.size(); i++) {
int temp_x = branch_pixels[i].x;
int temp_y = branch_pixels[i].y;
int temp_adc = branch_pixels[i].adc;
auto const temp_x = branch_pixels[i].x;
auto const temp_y = branch_pixels[i].y;
auto const temp_adc = branch_pixels[i].adc;
SiPixelCluster::PixelPos newpix(temp_x, temp_y);
cluster.add(newpix, temp_adc);
}
Expand Down
20 changes: 12 additions & 8 deletions RecoLocalTracker/SiPixelClusterizer/plugins/PixelThresholdClusterizerForBricked.cc
View file
Expand Up @@ -19,7 +19,8 @@
#include <vector>
#include <iostream>
#include <atomic>
using namespace std;
#include <algorithm>
#include <limits>

//----------------------------------------------------------------------------
//! Constructor:
Expand Down Expand Up @@ -119,16 +120,18 @@ void PixelThresholdClusterizerForBricked::clusterizeDetUnitT(const T& input,
SiPixelCluster PixelThresholdClusterizerForBricked::make_cluster_bricked(
const SiPixelCluster::PixelPos& pix, edmNew::DetSetVector<SiPixelCluster>::FastFiller& output, bool isbarrel) {
//First we acquire the seeds for the clusters
int seed_adc;
stack<SiPixelCluster::PixelPos, vector<SiPixelCluster::PixelPos> > dead_pixel_stack;
std::stack<SiPixelCluster::PixelPos, std::vector<SiPixelCluster::PixelPos> > dead_pixel_stack;

//The individual modules have been loaded into a buffer.
//After each pixel has been considered by the clusterizer, we set the adc count to 1
//to mark that we have already considered it.
//The only difference between dead/noisy pixels and standard ones is that for dead/noisy pixels,
//We consider the charge of the pixel to always be zero.

seed_adc = theBuffer(pix.row(), pix.col());
// Note: each ADC value is limited here to 65535 (std::numeric_limits<uint16_t>::max),
// as it is later stored as uint16_t in SiPixelCluster and PixelClusterizerBase/AccretionCluster
// (reminder: ADC values here may be expressed in number of electrons)
uint16_t seed_adc = std::min(theBuffer(pix.row(), pix.col()), int(std::numeric_limits<uint16_t>::max()));
theBuffer.set_adc(pix, 1);

AccretionCluster acluster;
Expand Down Expand Up @@ -167,7 +170,8 @@ SiPixelCluster PixelThresholdClusterizerForBricked::make_cluster_bricked(
for (auto c = LowerAccLimity; c < UpperAccLimity; ++c) {
if (theBuffer(r, c) >= thePixelThreshold) {
SiPixelCluster::PixelPos newpix(r, c);
if (!acluster.add(newpix, theBuffer(r, c)))
auto const newpix_adc = std::min(theBuffer(r, c), int(std::numeric_limits<uint16_t>::max()));
if (!acluster.add(newpix, newpix_adc))
goto endClus;
if (isbarrel)
edm::LogInfo("make_cluster_bricked()") << "add" << r << c << theBuffer(r, c);
Expand Down Expand Up @@ -210,9 +214,9 @@ SiPixelCluster PixelThresholdClusterizerForBricked::make_cluster_bricked(
//This loop adds the second cluster to the first.
const std::vector<SiPixelCluster::Pixel>& branch_pixels = second_cluster.pixels();
for (unsigned int i = 0; i < branch_pixels.size(); i++) {
int temp_x = branch_pixels[i].x;
int temp_y = branch_pixels[i].y;
int temp_adc = branch_pixels[i].adc;
auto const temp_x = branch_pixels[i].x;
auto const temp_y = branch_pixels[i].y;
auto const temp_adc = branch_pixels[i].adc;
SiPixelCluster::PixelPos newpix(temp_x, temp_y);
cluster.add(newpix, temp_adc);
}
Expand Down
24 changes: 15 additions & 9 deletions RecoLocalTracker/SiPixelClusterizer/plugins/gpuCalibPixel.h
View file
Expand Up @@ -3,6 +3,8 @@

#include <cstdint>
#include <cstdio>
#include <algorithm>
#include <limits>

#include "CUDADataFormats/SiPixelCluster/interface/gpuClusteringConstants.h"
#include "CondFormats/SiPixelObjects/interface/SiPixelGainForHLTonGPU.h"
Expand Down Expand Up @@ -69,7 +71,7 @@ namespace gpuCalibPixel {
float offset = id[i] < 96 ? VCaltoElectronOffset_L1 : VCaltoElectronOffset;
vcal = vcal * conversionFactor + offset;
}
adc[i] = std::max(100, int(vcal));
adc[i] = std::clamp(int(vcal), 100, int(std::numeric_limits<uint16_t>::max()));
}
}
}
Expand All @@ -96,24 +98,28 @@ namespace gpuCalibPixel {

constexpr int mode = (Phase2ReadoutMode < -1 ? -1 : Phase2ReadoutMode);

int adc_int = adc[i];

if constexpr (mode < 0)
adc[i] = int(adc[i] * ElectronPerADCGain);
adc_int = int(adc_int * ElectronPerADCGain);
else {
if (adc[i] < Phase2KinkADC)
adc[i] = int((adc[i] - 0.5) * ElectronPerADCGain);
if (adc_int < Phase2KinkADC)
adc_int = int((adc_int - 0.5) * ElectronPerADCGain);
else {
constexpr int8_t dspp = (Phase2ReadoutMode < 10 ? Phase2ReadoutMode : 10);
constexpr int8_t ds = int8_t(dspp <= 1 ? 1 : (dspp - 1) * (dspp - 1));

adc[i] -= (Phase2KinkADC - 1);
adc[i] *= ds;
adc[i] += (Phase2KinkADC - 1);
adc_int -= (Phase2KinkADC - 1);
adc_int *= ds;
adc_int += (Phase2KinkADC - 1);

adc[i] = uint16_t((adc[i] + 0.5 * ds) * ElectronPerADCGain);
adc_int = ((adc_int + 0.5 * ds) * ElectronPerADCGain);
}

adc[i] += int(Phase2DigiBaseline);
adc_int += int(Phase2DigiBaseline);
}

adc[i] = std::min(adc_int, int(std::numeric_limits<uint16_t>::max()));
}
}

Expand Down

0 comments on commit 3e286ef

Please sign in to comment.