Skip to content

[PWGJE] Bugfixes in JE tasks - #17903

Merged
nzardosh merged 8 commits into
AliceO2Group:masterfrom
mhwang285:fix-kappa
Sep 16, 2026
Merged

nzardosh merged 8 commits into
AliceO2Group:masterfrom
mhwang285:fix-kappa

Conversation

@mhwang285

Copy link
Copy Markdown
Contributor
  • Kappa and alpha were not being used in some angularity calculations
  • Resolved some O2 linter issues (most but not all)
  • Fixed two mislabeled histogram axes in jetSpectraCharged.cxx

@github-actions github-actions Bot added the pwgje label Sep 14, 2026
@github-actions github-actions Bot changed the title Bugfixes in JE tasks [PWGJE] Bugfixes in JE tasks Sep 14, 2026
@github-actions

github-actions Bot commented Sep 14, 2026

Copy link
Copy Markdown

O2 linter results: ❌ 9 errors, ⚠️ 3 warnings, 🔕 2 disabled

@alibuild

Copy link
Copy Markdown
Collaborator

Error while checking build/O2Physics/code-check for 0fdf62c at 2026-09-14 10:39:

## sw/BUILD/O2Physics-code-check-latest/log
--
========== List of issues found ==========
++ echo 'Found 7 errors and 33 warnings.'
Found 7 errors and 33 warnings.
++ [[ 7 -gt 0 ]]
++ cat /sw/BUILD/3e8892830ac299ff9f55a0018cd9f1299d603846/O2Physics-code-check/errors.txt
PWGJE/Tasks/jetFinderQA.cxx:34:1: error: included header TMath.h is not used directly [misc-include-cleaner,-warnings-as-errors]
PWGJE/Tasks/jetFinderQA.cxx:35:1: error: included header TMathBase.h is not used directly [misc-include-cleaner,-warnings-as-errors]
PWGJE/Tasks/jetFinderQA.cxx:483:66: error: no header providing "o2::constants::math::PI" is directly included [misc-include-cleaner,-warnings-as-errors]
PWGJE/Tasks/jetFinderQA.cxx:821:74: error: no header providing "o2::constants::math::TwoPI" is directly included [misc-include-cleaner,-warnings-as-errors]
PWGJE/Tasks/jetFinderQA.cxx:1299:13: error: Variable 'pt' with floating point type 'double' should not be used as a loop counter [clang-analyzer-security.FloatLoopCounter,-warnings-as-errors]
PWGJE/Tasks/jetFinderQA.cxx:1305:13: error: Variable 'pt' with floating point type 'double' should not be used as a loop counter [clang-analyzer-security.FloatLoopCounter,-warnings-as-errors]
PWGJE/Tasks/jetFinderQA.cxx:1311:13: error: Variable 'pt' with floating point type 'double' should not be used as a loop counter [clang-analyzer-security.FloatLoopCounter,-warnings-as-errors]
++ [[ 33 -gt 0 ]]
++ cat /sw/BUILD/3e8892830ac299ff9f55a0018cd9f1299d603846/O2Physics-code-check/warnings.txt
PWGJE/Tasks/jetFinderQA.cxx:45:10: warning: inclusion of deprecated C++ header 'math.h'; consider using 'cmath' instead [modernize-deprecated-headers]
PWGJE/Tasks/jetFinderQA.cxx:116:32: warning: implicit conversion 'double' -> 'bool' [readability-implicit-bool-conversion]
PWGJE/Tasks/jetFinderQA.cxx:116:32: warning: narrowing conversion from constant 'double' to 'bool' [bugprone-narrowing-conversions]
PWGJE/Tasks/jetFinderQA.cxx:117:31: warning: implicit conversion 'double' -> 'bool' [readability-implicit-bool-conversion]
PWGJE/Tasks/jetFinderQA.cxx:117:31: warning: narrowing conversion from constant 'double' to 'bool' [bugprone-narrowing-conversions]
PWGJE/Tasks/jetFinderQA.cxx:118:32: warning: implicit conversion 'double' -> 'bool' [readability-implicit-bool-conversion]
PWGJE/Tasks/jetFinderQA.cxx:118:32: warning: narrowing conversion from constant 'double' to 'bool' [bugprone-narrowing-conversions]
PWGJE/Tasks/jetFinderQA.cxx:825:77: warning: redundant explicit casting to the same type 'float' as the sub-expression, remove this casting [readability-redundant-casting]
PWGJE/Tasks/jetFinderQA.cxx:838:119: warning: redundant explicit casting to the same type 'float' as the sub-expression, remove this casting [readability-redundant-casting]
PWGJE/Tasks/jetFinderQA.cxx:849:98: warning: redundant explicit casting to the same type 'float' as the sub-expression, remove this casting [readability-redundant-casting]
PWGJE/Tasks/jetFinderQA.cxx:857:94: warning: redundant explicit casting to the same type 'float' as the sub-expression, remove this casting [readability-redundant-casting]
PWGJE/Tasks/jetFinderQA.cxx:864:81: warning: redundant explicit casting to the same type 'float' as the sub-expression, remove this casting [readability-redundant-casting]
PWGJE/Tasks/jetFinderQA.cxx:881:119: warning: redundant explicit casting to the same type 'float' as the sub-expression, remove this casting [readability-redundant-casting]
PWGJE/Tasks/jetFinderQA.cxx:912:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetFinderQA.cxx:922:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetFinderQA.cxx:932:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetFinderQA.cxx:942:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetFinderQA.cxx:946:174: warning: repeated branch body in conditional chain [bugprone-branch-clone]
PWGJE/Tasks/jetFinderQA.cxx:948:7: warning: do not use 'else' after 'return' [readability-else-after-return]
PWGJE/Tasks/jetFinderQA.cxx:1545:49: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr]
PWGJE/Tasks/jetSpectraCharged.cxx:295:9: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]
PWGJE/Tasks/jetSpectraCharged.cxx:358:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:368:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:378:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:419:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:429:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:439:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:455:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:465:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:475:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:485:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:1027:31: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]
PWGJE/Tasks/jetSpectraCharged.cxx:1110:31: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]
[0 more errors; see full log]

Full log here.

@nzardosh nzardosh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Dear Tucker, thanks for making the changes. I have left a couple of comments.

For a future note it is best not to change the analysis tasks written by other analysers, even if the changes are cosmetic (unless discussed with them first, in which case please disregard this) as they might be making changes of their own in their local branches. @aimeric-landou @joonsukbae could you please check and make sure you are ok with the changes proposed.

Filter eventCuts = (nabs(aod::jcollision::posZ) < vertexZCut &&
((checkCentFT0M ? aod::jcollision::centFT0M : aod::jcollision::centFT0C) >= centralityMin) &&
((checkCentFT0M ? aod::jcollision::centFT0M : aod::jcollision::centFT0C) < centralityMax));
PresliceUnsorted<soa::Filtered<aod::JetCollisionsMCD>> CollisionsPerMCPCollision = aod::jmccollisionlb::mcCollisionId;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

For these could you put them back please? even though it goes against the convention in most of the framework I use capitals for preslices to differentiate them. We might decide to change this globally for later but for now would be nice to keep it consistent

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, recapitalized now in 81405af

Comment thread PWGJE/Tasks/jetSpectraCharged.cxx Outdated
angularityMcd += std::pow(constituent.pt(), kappa) * std::pow(jetutilities::deltaR(jetMCD, constituent), alpha);
angularityMcd += std::pow(constituent.pt(), kappa) * std::pow(jetutilities::deltaR(jetMCD, constituent) / (jetMCD.r() / 100.f), alpha);
}
angularityMcd /= (corrBasejetpt * (jetMCD.r() / 100.f));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

in line 746 there is a division by the radius. There shouldnt be another one here i think?

@mhwang285 mhwang285 Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah thanks for catching that, I fixed 746 but not 748, both lines are fixed now. I also had to change corrTagjetpt, corrBasejetpt, and dcorrpt to floats so that we can use std::pow with a kappa (which is float)

@alibuild

Copy link
Copy Markdown
Collaborator

Error while checking build/O2Physics/code-check for 7658b87 at 2026-09-15 00:52:

## sw/BUILD/O2Physics-code-check-latest/log
--
========== List of issues found ==========
++ echo 'Found 3 errors and 32 warnings.'
Found 3 errors and 32 warnings.
++ [[ 3 -gt 0 ]]
++ cat /sw/BUILD/e73c790b3a9d8070b41e8d403705aec919e166d7/O2Physics-code-check/errors.txt
PWGJE/Tasks/jetFinderQA.cxx:1296:13: error: Variable 'pt' with floating point type 'double' should not be used as a loop counter [clang-analyzer-security.FloatLoopCounter,-warnings-as-errors]
PWGJE/Tasks/jetFinderQA.cxx:1302:13: error: Variable 'pt' with floating point type 'double' should not be used as a loop counter [clang-analyzer-security.FloatLoopCounter,-warnings-as-errors]
PWGJE/Tasks/jetFinderQA.cxx:1308:13: error: Variable 'pt' with floating point type 'double' should not be used as a loop counter [clang-analyzer-security.FloatLoopCounter,-warnings-as-errors]
++ [[ 32 -gt 0 ]]
++ cat /sw/BUILD/e73c790b3a9d8070b41e8d403705aec919e166d7/O2Physics-code-check/warnings.txt
PWGJE/Tasks/jetFinderQA.cxx:113:32: warning: implicit conversion 'double' -> 'bool' [readability-implicit-bool-conversion]
PWGJE/Tasks/jetFinderQA.cxx:113:32: warning: narrowing conversion from constant 'double' to 'bool' [bugprone-narrowing-conversions]
PWGJE/Tasks/jetFinderQA.cxx:114:31: warning: implicit conversion 'double' -> 'bool' [readability-implicit-bool-conversion]
PWGJE/Tasks/jetFinderQA.cxx:114:31: warning: narrowing conversion from constant 'double' to 'bool' [bugprone-narrowing-conversions]
PWGJE/Tasks/jetFinderQA.cxx:115:32: warning: implicit conversion 'double' -> 'bool' [readability-implicit-bool-conversion]
PWGJE/Tasks/jetFinderQA.cxx:115:32: warning: narrowing conversion from constant 'double' to 'bool' [bugprone-narrowing-conversions]
PWGJE/Tasks/jetFinderQA.cxx:822:77: warning: redundant explicit casting to the same type 'float' as the sub-expression, remove this casting [readability-redundant-casting]
PWGJE/Tasks/jetFinderQA.cxx:835:119: warning: redundant explicit casting to the same type 'float' as the sub-expression, remove this casting [readability-redundant-casting]
PWGJE/Tasks/jetFinderQA.cxx:846:98: warning: redundant explicit casting to the same type 'float' as the sub-expression, remove this casting [readability-redundant-casting]
PWGJE/Tasks/jetFinderQA.cxx:854:94: warning: redundant explicit casting to the same type 'float' as the sub-expression, remove this casting [readability-redundant-casting]
PWGJE/Tasks/jetFinderQA.cxx:861:81: warning: redundant explicit casting to the same type 'float' as the sub-expression, remove this casting [readability-redundant-casting]
PWGJE/Tasks/jetFinderQA.cxx:878:119: warning: redundant explicit casting to the same type 'float' as the sub-expression, remove this casting [readability-redundant-casting]
PWGJE/Tasks/jetFinderQA.cxx:909:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetFinderQA.cxx:919:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetFinderQA.cxx:929:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetFinderQA.cxx:939:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetFinderQA.cxx:943:174: warning: repeated branch body in conditional chain [bugprone-branch-clone]
PWGJE/Tasks/jetFinderQA.cxx:945:7: warning: do not use 'else' after 'return' [readability-else-after-return]
PWGJE/Tasks/jetFinderQA.cxx:1542:49: warning: redundant boolean literal supplied to boolean operator [readability-simplify-boolean-expr]
PWGJE/Tasks/jetSpectraCharged.cxx:295:9: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]
PWGJE/Tasks/jetSpectraCharged.cxx:358:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:368:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:378:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:419:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:429:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:439:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:455:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:465:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:475:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:485:22: warning: statement should be inside braces [readability-braces-around-statements]
PWGJE/Tasks/jetSpectraCharged.cxx:1027:31: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]
PWGJE/Tasks/jetSpectraCharged.cxx:1110:31: warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]
++ [[ 3 -gt 0 ]]
++ exit 1
--

Full log here.

@joonsukbae

Copy link
Copy Markdown
Contributor

Dear Tucker, thanks for making the changes. I have left a couple of comments.

For a future note it is best not to change the analysis tasks written by other analysers, even if the changes are cosmetic (unless discussed with them first, in which case please disregard this) as they might be making changes of their own in their local branches. @aimeric-landou @joonsukbae could you please check and make sure you are ok with the changes proposed.

Dear @mhwang285 and @nzardosh, it looks good to me. The type/name changes and the angularity expansion also look good. Thank you also for catching the switched MCD vs. MCP axis labels.

@nzardosh
nzardosh self-requested a review September 16, 2026 10:58
@nzardosh
nzardosh merged commit 322aaa6 into AliceO2Group:master Sep 16, 2026
11 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

4 participants