[PWGHF,Tools] Address Bug tracker issues in LcToPKPi-related files #14358

lubynets · 2025-12-28T22:03:23Z

Clang-tidy issues present in workflows related to $\Lambda^+_c\rightarrow pK\pi^+$ reconstruction are fixed.
I am not 100% sure that some of the narrowing conversion issues are addressed in proper places (e.g. change of the Configurable double->float when it is consumed by function with float parameter; or issues with int8_t values), therefore an attentive look and feedback is appreciated.
UPD: addressed (part of) issues in Tools/KFparticle/KFUtilities.h - clang-tidy and O2 linter.

github-actions · 2025-12-28T22:03:39Z

O2 linter results: ❌ 37 errors, ⚠️ 83 warnings, 🔕 11 disabled

vkucera · 2025-12-30T15:00:48Z

PWGHF/TableProducer/treeCreatorLcToPKPi.cxx

  /// \param isMc boolean flag whether MC or data is processed
  template <int ReconstructionType>
-  void reserveTables(size_t candidatesSize, bool isMc)
+  void reserveTables(int64_t candidatesSize, bool isMc)


Why is this needed? Doesn't the reserve method take an unsigned type?

Yes, the reserve() function of the WritingCursor struct (which is a base struct for Produces), has an int64_t parameter.
And the clang-tidy confirms that: PWGHF/TableProducer/treeCreatorLcToPKPi.cxx:551:34: warning: narrowing conversion from 'size_t' (aka 'unsigned long') to signed type 'int64_t' (aka 'long') is implementation-defined [bugprone-narrowing-conversions].

@aalkin Why is that?

The reserve() method did not change from 2019(?), and arrow still uses unsigned int64_t explicitly for its sizes. Regardless, the explicit ints are preferred to size_t which is implementation-dependent.

So why does the reserve method not use the uint64_t instead of int64_t?

Because it calls arrow's method that expects int64_t and thus it was used when the method was implemented. There is no intent behind it.

So this mismatch between signed and unsigned type is coming already from arrow?

lubynets · 2026-01-04T20:38:18Z

Help wanted

The clang-tidy tracker produces a "narrowing conversion from 'double' to 'float' " warning to the Tools/KFparticle/KFUtilities.h line 299 (and a few similar warnings), while both r.h.s. and l.h.s. of the assignment are float. Is it a bug of clang-tidy or am I overlooking something?
Is it possible to use O2 constants in order to determine particle's mass knowing its PDG code? - in order to get rid of TDatabasePDG here?

vkucera · 2026-01-06T09:12:04Z

clangd agrees with Clang-Tidy. sqrt returns double. If I change it to std::sqrt it returns float.
O2 linter tells you what to do. If you know the PDG code at compile time, use a header constant, otherwise, use the service.

vkucera · 2026-01-06T09:23:04Z

PWGHF/TableProducer/candidateCreator3Prong.cxx

+        kfCandPKPi.SetNonlinearMassConstraint(createLc ? static_cast<float>(MassLambdaCPlus) : static_cast<float>(MassXiCPlus));
+        kfCandPiKP.SetNonlinearMassConstraint(createLc ? static_cast<float>(MassLambdaCPlus) : static_cast<float>(MassXiCPlus));


The SetNonlinearMassConstraint() has a float parameter, while particle mass O2 constants are double.
BTW clang-tidy generates warning only when the conditional operator is used, otherwise (few lines below highlighted) there is no warning.

That's what I was wondering, since below you still pass double values.

I think the key difference is that below I pass constant expressions, and compiler does not consider it as narrowing conversion since this particular double value fits into float range.
And the expression with conditional operator is not constant (although 2-nd and 3-d operands are constexpr).

vkucera · 2026-01-06T09:26:18Z

PWGHF/TableProducer/candidateCreator3Prong.cxx

          }
          if (indexRec > -1) {
-            flagChannelMain = sign * DecayChannelMain::DplusToPiKPi;
+            flagChannelMain = sign * static_cast<int8_t>(DecayChannelMain::DplusToPiKPi);


Useless. It already is int8_t.

Fixed by commit.

I don't think so. Both sign and DplusToPiKPi are int8_t. Their product is int8_t as well. You don't need the cast at all.

Yes, both sign and DplusToPiKPi are int8_t, but their product is promoted to int. I checked it both in ROOT interpreter mode and as a compiled code:

#include <iostream> int main() { int8_t a{2}; int8_t b{3}; auto c = a * b; std::cout << "Type of a: " << typeid(a).name() << "\n"; std::cout << "Type of b: " << typeid(b).name() << "\n"; std::cout << "Type of c: " << typeid(c).name() << "\n"; return 0; }

Yes, indeed, sorry. Actually, both operands are promoted to int before the multiplication, so it's the result that has to be casted to the LHC type, as you did correctly. Thanks!

vkucera · 2026-01-06T09:27:34Z

PWGHF/TableProducer/candidateCreator3Prong.cxx

-  Configurable<double> maxR{"maxR", 200., "reject PCA's above this radius"};
-  Configurable<double> maxDZIni{"maxDZIni", 4., "reject (if>0) PCA candidate if tracks DZ exceeds threshold"};
-  Configurable<double> minParamChange{"minParamChange", 1.e-3, "stop iterations if largest change of any X is smaller than this"};
-  Configurable<double> minRelChi2Change{"minRelChi2Change", 0.9, "stop iterations is chi2/chi2old > this"};
+  Configurable<float> maxR{"maxR", 200., "reject PCA's above this radius"};
+  Configurable<float> maxDZIni{"maxDZIni", 4., "reject (if>0) PCA candidate if tracks DZ exceeds threshold"};
+  Configurable<float> minParamChange{"minParamChange", 1.e-3, "stop iterations if largest change of any X is smaller than this"};
+  Configurable<float> minRelChi2Change{"minRelChi2Change", 0.9, "stop iterations is chi2/chi2old > this"};


This will invalidate existing wagon configuration. Not sure we want that.

So will it be a better solution to revert Configurables' types to double and static_cast to float functions' arguments?

So, we define Configurables with double precision which we never use, since immediately cast it to float. Is it worth fixing once with taking care of wagons adjusting (also once, and the place of concern is well known)?
I will take the responsibility to wide-post in the HF chat the announcement and ask service wagons owners to do adjustment.

Tools/KFparticle/KFUtilities.h

lubynets · 2026-01-07T11:56:09Z

clangd agrees with Clang-Tidy. sqrt returns double. If I change it to std::sqrt it returns float.

O2 linter tells you what to do. If you know the PDG code at compile time, use a header constant, otherwise, use the service.

Hi @vkucera, thanks for your feedback!
2. The function where mass is required, is a free (non-member) function. Will not be a problem from the CPU time point of view to instantiate the Service<o2::framework::O2DatabasePDG> in the function?
And if that's a problem, will making it static solve it?

vkucera · 2026-01-07T16:14:21Z

You should pass the pointer to the object.

lubynets · 2026-01-07T16:34:34Z

Do you mean the Service should be instantiated in the global scope, and the pointer to it should be passed as an argument to the function which needs particle's mass?

No, it should be instantiated as a member of a workflow struct.

UPD: Done (O.L.)

…headers

Tools/KFparticle/KFUtilities.h

address clang-tidy issues

8998841

github-actions bot added the pwghf PWG-HF label Dec 28, 2025

github-actions bot changed the title ~~Address clang-tidy issues in LcToPKPi-related files~~ [PWGHF] Address clang-tidy issues in LcToPKPi-related files Dec 28, 2025

vkucera reviewed Dec 30, 2025

View reviewed changes

lubynets added 2 commits January 4, 2026 20:53

address clang-tidy issues in Tools/KFUtilities.h

254a528

get rid of magic constants in Tools/KFUtilities.h

c4a8252

github-actions bot added the tools label Jan 4, 2026

lubynets changed the title ~~[PWGHF] Address clang-tidy issues in LcToPKPi-related files~~ [PWGHF] Address Bug tracker issues in LcToPKPi-related files Jan 4, 2026

KFUtilities.h: use lowerCamelCase for names of variables

9e4e386

vkucera reviewed Jan 6, 2026

View reviewed changes

Tools/KFparticle/KFUtilities.h Show resolved Hide resolved

static_cast int to int8_t with a proper argument

c4232b1

lubynets added 4 commits January 7, 2026 18:58

linter concession: KFParticle style of preprocessor macro

184d2c6

use Service pdg instead of TDatabasePDG

8d5291b

qaKFParticle.cxx: put HomogeneousField directive before including KF …

f79b8da

…headers

qaKFParticle.cxx: use std:: prefix for names from the std namespace

904d26c

vkucera reviewed Jan 8, 2026

View reviewed changes

Tools/KFparticle/KFUtilities.h Outdated Show resolved Hide resolved

use std::hypot for vector norm calculation

12e780f

lubynets changed the title ~~[PWGHF] Address Bug tracker issues in LcToPKPi-related files~~ [PWGHF,Tools] Address Bug tracker issues in LcToPKPi-related files Jan 8, 2026

		kfCandPKPi.SetNonlinearMassConstraint(createLc ? static_cast<float>(MassLambdaCPlus) : static_cast<float>(MassXiCPlus));
		kfCandPiKP.SetNonlinearMassConstraint(createLc ? static_cast<float>(MassLambdaCPlus) : static_cast<float>(MassXiCPlus));

[PWGHF,Tools] Address Bug tracker issues in LcToPKPi-related files #14358

Are you sure you want to change the base?

[PWGHF,Tools] Address Bug tracker issues in LcToPKPi-related files #14358

Conversation

lubynets commented Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lubynets commented Jan 4, 2026

Uh oh!

vkucera commented Jan 6, 2026

Uh oh!

vkucera Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vkucera Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lubynets commented Jan 7, 2026

Uh oh!

vkucera commented Jan 7, 2026

Uh oh!

lubynets commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

lubynets commented Dec 28, 2025 •

edited

Loading

github-actions bot commented Dec 28, 2025 •

edited

Loading

vkucera Jan 6, 2026 •

edited

Loading

vkucera Jan 7, 2026 •

edited

Loading

lubynets commented Jan 7, 2026 •

edited

Loading