Skip to content
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

guard against uint16_t overflows of Pixel ADC values [12_3_X] #38149

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
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
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