-
Notifications
You must be signed in to change notification settings - Fork 13
Conversation
/run standalone |
The PR was built and ran successfully in standalone mode. Here are some of the comparison plots. The full set of validation and comparison plots can be found here. |
The PR was built and ran successfully with CMSSW. Here are some plots. OOTB All TracksThe full set of validation and comparison plots can be found here. |
@@ -2577,7 +2575,7 @@ namespace SDL { | |||
|
|||
rzChiSquared = computePT5RZChiSquared(acc, modulesInGPU, lowerModuleIndices, rtPix, zPix, rts, zs); | |||
|
|||
if (pixelRadius < 5.0f * kR1GeVf) { | |||
if (/*pixelRadius*/ 0 < 5.0f * kR1GeVf) { // FIXME: pixelRadius is not defined yet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@YonsiG this is the part I was talking about during the meeting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for finding it Slava! I'm just curious, why do we want to only use the rzChiSquared only for < 5GeV? For larger pt, in principal, the track on rz should be very straight line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, in rz computations hopefully do not explode numerically and we could safely apply the cut everywhere (well, we do already, by virtue of the bug here).
Note though that computePT5RZChiSquared
is not using the helix correction.
Why? Doesn't this lead to inefficiency at low pt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Slava, thank you for your explanation! I don't remember there is a reason to keep the linear constraint for pT5 r-z. I think it's just we haven't applied the helix here yet. Can add it on my to-do list if necessary.
I think that this is ready for review; I guess we can pick a reviewer in the meeting tomorrow, unless someone volunteers sooner |
When compiling for cpu I get one last warning. (It's pretty easy to fix) In file included from Kernels.h:9,
from Event.h:9,
from Event.cc:1:
In function 'void SDL::addQuintupletToMemory(triplets&, quintuplets&, unsigned int, unsigned int, uint16_t&, uint16_t&, uint16_t&, uint16_t&, uint16_t&, float&, float&, float&, float&, float&, float&, float&, float&, float&, float, float, float, float, uint8_t, unsigned int, bool)',
inlined from 'void SDL::createQuintupletsInGPUv2::operator()(const TAcc&, SDL::modules, SDL::miniDoublets, SDL::segments, SDL::triplets, SDL::quintuplets, SDL::objectRanges, uint16_t) const [with TAcc = alpaka::AccCpuSerial<std::integral_constant<long unsigned int, 3>, long unsigned int>]' at Quintuplet.h:3104:40:
Quintuplet.h:206:52: warning: 'rzChiSquared' may be used uninitialized [-Wmaybe-uninitialized]
206 | quintupletsInGPU.rzChiSquared[quintupletIndex] = rzChiSquared;
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
Quintuplet.h: In function 'void SDL::createQuintupletsInGPUv2::operator()(const TAcc&, SDL::modules, SDL::miniDoublets, SDL::segments, SDL::triplets, SDL::quintuplets, SDL::objectRanges, uint16_t) const [with TAcc = alpaka::AccCpuSerial<std::integral_constant<long unsigned int, 3>, long unsigned int>]':
Quintuplet.h:3051:103: note: 'rzChiSquared' was declared here
3051 | float innerRadius, outerRadius, bridgeRadius, regressionG, regressionF, regressionRadius, rzChiSquared,
| But this is really nice. It's great so see such a clean compilation log. |
I'm curious why I don't get this warning |
are you compiling in a special setup without |
…osed to initialize; fix maybe-uninitialized for builds without DNN and without USE_RZCHI2
please check after 8f67f82 |
/run standalone |
The PR was built and ran successfully in standalone mode. Here are some of the comparison plots. The full set of validation and comparison plots can be found here. |
The PR was built and ran successfully with CMSSW. Here are some plots. OOTB All TracksThe full set of validation and comparison plots can be found here. |
Oh yeah, I was testing it directly running make and I forgot to include some of the extra flags. There are no warnings now regardless of the flags. Now that I think about it, I also need to fix LSTCore so that it uses all these extra flags. Thank you, @slava77! |
I just noticed that the ROCm compilation in CMSSW seems to use a couple of extra warning flags, but I guess we can worry about that later. |
This attempts to address compilation warnings in SDL/
alpaka::memset
3rd argument is a byte (uint8)I think there is a bug in
runPixelQuintupletDefaultAlgo
where the cut onpassPT5RZChiSquaredCuts
looks like it needs to be applied only for pLS pt < 5 GeV, but it's apparently applied to all because pLS pt at that point of the code is not defined.