SegmentLinking / TrackLooper

Apache License 2.0
5 stars 14 forks source link

Warnings cleanup #383

Closed slava77 closed 7 months ago

slava77 commented 7 months ago

This attempts to address compilation warnings in SDL/

I think there is a bug in runPixelQuintupletDefaultAlgo where the cut on passPT5RZChiSquaredCuts 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.

slava77 commented 7 months ago

/run standalone /run cmssw

github-actions[bot] commented 7 months ago

The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.

Efficiency vs pT comparison Efficiency vs eta comparison
Fake rate vs pT comparison Fake rate vs eta comparison
Duplicate rate vs pT comparison Duplicate rate vs eta comparison

The full set of validation and comparison plots can be found here.

github-actions[bot] commented 7 months ago

The PR was built and ran successfully with CMSSW. Here are some plots.

OOTB All Tracks
Efficiency and fake rate vs pT, eta, and phi

The full set of validation and comparison plots can be found here.

slava77 commented 7 months ago

I think that this is ready for review; I guess we can pick a reviewer in the meeting tomorrow, unless someone volunteers sooner

ariostas commented 7 months ago

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.

slava77 commented 7 months ago

When compiling for cpu I get one last warning. (It's pretty easy to fix)

I'm curious why I don't get this warning

slava77 commented 7 months ago

When compiling for cpu I get one last warning. (It's pretty easy to fix)

I'm curious why I don't get this warning

are you compiling in a special setup without DUSE_RZCHI2 ?

slava77 commented 7 months ago

When compiling for cpu I get one last warning. (It's pretty easy to fix)

please check after 8f67f82

slava77 commented 7 months ago

/run standalone /run cmssw

github-actions[bot] commented 7 months ago

The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.

Efficiency vs pT comparison Efficiency vs eta comparison
Fake rate vs pT comparison Fake rate vs eta comparison
Duplicate rate vs pT comparison Duplicate rate vs eta comparison

The full set of validation and comparison plots can be found here.

github-actions[bot] commented 7 months ago

The PR was built and ran successfully with CMSSW. Here are some plots.

OOTB All Tracks
Efficiency and fake rate vs pT, eta, and phi

The full set of validation and comparison plots can be found here.

ariostas commented 7 months ago

are you compiling in a special setup without DUSE_RZCHI2 ?

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!

ariostas commented 7 months ago

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.