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.

Sign up for GitHub

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

Further clean up the CA ntuplet generator #610

Merged
merged 1 commit into from
Mar 24, 2021
Merged

Further clean up the CA ntuplet generator #610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 20 additions & 13 deletions RecoPixelVertexing/PixelTriplets/plugins/CAHitNtupletGeneratorKernels.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,18 @@ void CAHitNtupletGeneratorKernelsCPU::buildDoublets(HitsOnCPU const &hh, cudaStr
std::cout << "building Doublets out of " << nhits << " Hits" << std::endl;
#endif

// in principle we can use "nhits" to heuristically dimension the workspace...
// overkill to use template here (std::make_unique would suffice)
// device_isOuterHitOfCell_ = Traits:: template make_unique<GPUCACell::OuterHitOfCell[]>(cs, std::max(1U,nhits), stream);
device_isOuterHitOfCell_.reset(
(GPUCACell::OuterHitOfCell *)malloc(std::max(1U, nhits) * sizeof(GPUCACell::OuterHitOfCell)));
// use "nhits" to heuristically dimension the workspace

// no need to use the Traits allocations, since we know this is being compiled for the CPU
//device_isOuterHitOfCell_ = Traits::template make_unique<GPUCACell::OuterHitOfCell[]>(std::max(1U, nhits), stream);
device_isOuterHitOfCell_ = std::make_unique<GPUCACell::OuterHitOfCell[]>(std::max(1U, nhits));
Comment on lines -21 to +25
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I've fixed the commented out allocations as well (at least, they compile now), and I've changed the used ones from malloc to make_unique (to be on the safe side, in case new were to do something different than plain malloc).
@VinInn do you prefer to keep the commented out version ?

Copy link

@VinInn VinInn Mar 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fine for the time being. Once we will have an agreed heterogeneous allocator it will be changed together all other instances....

assert(device_isOuterHitOfCell_.get());

cellStorage_.reset((unsigned char *)malloc(caConstants::maxNumOfActiveDoublets * sizeof(GPUCACell::CellNeighbors) +
caConstants::maxNumOfActiveDoublets * sizeof(GPUCACell::CellTracks)));
auto cellStorageSize = caConstants::maxNumOfActiveDoublets * sizeof(GPUCACell::CellNeighbors) +
caConstants::maxNumOfActiveDoublets * sizeof(GPUCACell::CellTracks);
// no need to use the Traits allocations, since we know this is being compiled for the CPU
//cellStorage_ = Traits::template make_unique<unsigned char[]>(cellStorageSize, stream);
cellStorage_ = std::make_unique<unsigned char[]>(cellStorageSize);
device_theCellNeighborsContainer_ = (GPUCACell::CellNeighbors *)cellStorage_.get();
device_theCellTracksContainer_ = (GPUCACell::CellTracks *)(cellStorage_.get() + caConstants::maxNumOfActiveDoublets *
sizeof(GPUCACell::CellNeighbors));
Expand All @@ -38,17 +41,21 @@ void CAHitNtupletGeneratorKernelsCPU::buildDoublets(HitsOnCPU const &hh, cudaStr
device_theCellTracks_.get(),
device_theCellTracksContainer_);

// device_theCells_ = Traits:: template make_unique<GPUCACell[]>(cs, m_params.maxNumberOfDoublets_, stream);
device_theCells_.reset((GPUCACell *)malloc(sizeof(GPUCACell) * params_.maxNumberOfDoublets_));
// no need to use the Traits allocations, since we know this is being compiled for the CPU
//device_theCells_ = Traits::template make_unique<GPUCACell[]>(params_.maxNumberOfDoublets_, stream);
device_theCells_ = std::make_unique<GPUCACell[]>(params_.maxNumberOfDoublets_);
if (0 == nhits)
return; // protect against empty events

// FIXME avoid magic numbers
// take all layer pairs into account
auto nActualPairs = gpuPixelDoublets::nPairs;
if (!params_.includeJumpingForwardDoublets_)
nActualPairs = 15;
if (not params_.includeJumpingForwardDoublets_) {
// exclude forward "jumping" layer pairs
nActualPairs = gpuPixelDoublets::nPairsForTriplets;
}
if (params_.minHitsPerNtuplet_ > 3) {
nActualPairs = 13;
// for quadruplets, exclude all "jumping" layer pairs
nActualPairs = gpuPixelDoublets::nPairsForQuadruplets;
}

assert(nActualPairs <= gpuPixelDoublets::nPairs);
Expand Down
11 changes: 7 additions & 4 deletions RecoPixelVertexing/PixelTriplets/plugins/CAHitNtupletGeneratorKernels.cu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,15 @@ void CAHitNtupletGeneratorKernelsGPU::buildDoublets(HitsOnCPU const &hh, cudaStr
if (0 == nhits)
return; // protect against empty events

// FIXME avoid magic numbers
// take all layer pairs into account
auto nActualPairs = gpuPixelDoublets::nPairs;
if (!params_.includeJumpingForwardDoublets_)
nActualPairs = 15;
if (not params_.includeJumpingForwardDoublets_) {
// exclude forward "jumping" layer pairs
nActualPairs = gpuPixelDoublets::nPairsForTriplets;
}
if (params_.minHitsPerNtuplet_ > 3) {
nActualPairs = 13;
// for quadruplets, exclude all "jumping" layer pairs
nActualPairs = gpuPixelDoublets::nPairsForQuadruplets;
}

assert(nActualPairs <= gpuPixelDoublets::nPairs);
Expand Down
6 changes: 4 additions & 2 deletions RecoPixelVertexing/PixelTriplets/plugins/gpuPixelDoublets.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@

namespace gpuPixelDoublets {

constexpr int nPairs = 13 + 2 + 4;
constexpr int nPairsForQuadruplets = 13; // quadruplets require hits in all layers
constexpr int nPairsForTriplets = nPairsForQuadruplets + 2; // include barrel "jumping" layer pairs
constexpr int nPairs = nPairsForTriplets + 4; // include forward "jumping" layer pairs
static_assert(nPairs <= caConstants::maxNumberOfLayerPairs);

// start constants
// clang-format off

CONSTANT_VAR const uint8_t layerPairs[2 * nPairs] = {
0, 1, 0, 4, 0, 7, // BPIX1 (3)
1, 2, 1, 4, 1, 7, // BPIX2 (5)
1, 2, 1, 4, 1, 7, // BPIX2 (6)
4, 5, 7, 8, // FPIX1 (8)
2, 3, 2, 4, 2, 7, 5, 6, 8, 9, // BPIX3 & FPIX2 (13)
0, 2, 1, 3, // Jumping Barrel (15)
Expand Down