Repository navigation
Conversation
|
O2 linter results: ❌ 0 errors, |
axti98
marked this pull request as ready for review
November 11, 2026 16:59
axti98
requested review from
XiaozhiBai,
alibuild,
glegras,
iarsene,
lucamicheletti93,
mguilbau and
shreyasiacharya
as code owners
November 11, 2026 16:59
axti98
marked this pull request as draft
November 11, 2026 20:15
Collaborator
|
Error while checking build/O2Physics/code-check for 4cf94e2 at 2026-11-11 21:32: Full log here. |
axti98
marked this pull request as ready for review
November 12, 2026 07:06
Collaborator
|
Error while checking build/O2Physics/code-check for 5269ff8 at 2026-11-12 08:21: Full log here. |
Collaborator
|
Error while checking build/O2Physics/code-check for 803801d at 2026-11-12 12:39: Full log here. |
Collaborator
|
Error while checking build/O2Physics/staging for 190f7b7 at 2026-11-12 13:31: Full log here. |
Collaborator
|
Error while checking build/O2Physics/code-check for 190f7b7 at 2026-11-12 13:34: Full log here. |
Collaborator
|
Error while checking build/O2Physics/o2 for 190f7b7 at 2026-11-12 13:32: Full log here. |
Collaborator
|
Error while checking build/O2Physics/code-check for 906bec0 at 2026-11-12 16:03: Full log here. |
vkucera
reviewed
Nov 12, 2026
Comment on lines
+28
to
+30
| inline constexpr std::uint8_t Flag8 = 0u; | ||
| inline constexpr std::uint32_t Flag32 = 0u; | ||
| inline constexpr std::uint64_t Flag64 = 0ull; |
Collaborator
|
@axti98 Very nice! Thanks! |
iarsene
reviewed
Nov 12, 2026
Comment on lines
+2419
to
+2424
| values[kQ1X0A] = o2::dqconstants::InvalidValueI; | ||
| values[kQ1Y0A] = o2::dqconstants::InvalidValueI; | ||
| values[kQ1X0B] = o2::dqconstants::InvalidValueI; | ||
| values[kQ1Y0B] = o2::dqconstants::InvalidValueI; | ||
| values[kQ1X0C] = o2::dqconstants::InvalidValueI; | ||
| values[kQ1Y0C] = o2::dqconstants::InvalidValueI; |
Collaborator
There was a problem hiding this comment.
these should be InvalidValueF, since all of the VarManager values are floats
Comment on lines
+2442
to
+2453
| values[kQ3X0A] = o2::dqconstants::InvalidValueI; | ||
| values[kQ3Y0A] = o2::dqconstants::InvalidValueI; | ||
| values[kQ3X0B] = o2::dqconstants::InvalidValueI; | ||
| values[kQ3Y0B] = o2::dqconstants::InvalidValueI; | ||
| values[kQ3X0C] = o2::dqconstants::InvalidValueI; | ||
| values[kQ3Y0C] = o2::dqconstants::InvalidValueI; | ||
| values[kQ4X0A] = o2::dqconstants::InvalidValueI; | ||
| values[kQ4Y0A] = o2::dqconstants::InvalidValueI; | ||
| values[kQ4X0B] = o2::dqconstants::InvalidValueI; | ||
| values[kQ4Y0B] = o2::dqconstants::InvalidValueI; | ||
| values[kQ4X0C] = o2::dqconstants::InvalidValueI; | ||
| values[kQ4Y0C] = o2::dqconstants::InvalidValueI; |
Collaborator
There was a problem hiding this comment.
all of these should be InvalidValueF
Comment on lines
+2529
to
+2534
| values[kQ1X0A] = o2::dqconstants::InvalidValueI; | ||
| values[kQ1Y0A] = o2::dqconstants::InvalidValueI; | ||
| values[kQ1X0B] = o2::dqconstants::InvalidValueI; | ||
| values[kQ1Y0B] = o2::dqconstants::InvalidValueI; | ||
| values[kQ1X0C] = o2::dqconstants::InvalidValueI; | ||
| values[kQ1Y0C] = o2::dqconstants::InvalidValueI; |
Collaborator
There was a problem hiding this comment.
all of these should be InvalidValueF
Comment on lines
+2550
to
+2561
| values[kQ3X0A] = o2::dqconstants::InvalidValueI; | ||
| values[kQ3Y0A] = o2::dqconstants::InvalidValueI; | ||
| values[kQ3X0B] = o2::dqconstants::InvalidValueI; | ||
| values[kQ3Y0B] = o2::dqconstants::InvalidValueI; | ||
| values[kQ3X0C] = o2::dqconstants::InvalidValueI; | ||
| values[kQ3Y0C] = o2::dqconstants::InvalidValueI; | ||
| values[kQ4X0A] = o2::dqconstants::InvalidValueI; | ||
| values[kQ4Y0A] = o2::dqconstants::InvalidValueI; | ||
| values[kQ4X0B] = o2::dqconstants::InvalidValueI; | ||
| values[kQ4Y0B] = o2::dqconstants::InvalidValueI; | ||
| values[kQ4X0C] = o2::dqconstants::InvalidValueI; | ||
| values[kQ4Y0C] = o2::dqconstants::InvalidValueI; |
Collaborator
There was a problem hiding this comment.
all of these should be InvalidValueF
| @@ -2642,7 +2641,7 @@ void VarManager::FillEvent(T const& event, float* values) | |||
|
|
|||
| if constexpr ((fillMap & ReducedEventMC) > 0) { | |||
| values[kMCEventGeneratorId] = event.generatorsID(); | |||
| values[kMCEventGeneratorId] = -999; // to be added in reduced events | |||
| values[kMCEventGeneratorId] = o2::dqconstants::InvalidValueI; // to be added in reduced events | |||
|
|
||
| double det1 = t1.cYY() * t1.cZZ() - t1.cZY() * t1.cZY(); | ||
| double det2 = t2.cYY() * t2.cZZ() - t2.cZY() * t2.cZY(); | ||
| if ((det1 < 0) || (det2 < 0)) { | ||
| values[kQuadDCAsigXYZ] = -999; | ||
| values[kQuadDCAsigXYZ] = o2::dqconstants::InvalidValueI; |
Collaborator
There was a problem hiding this comment.
It should be InvalidValueF
Comment on lines
-657
to
-658
| kPVContributor, // This track has contributed to the collision vertex fit (see Framework/DataTypes) | ||
| kOrphanTrack, // Track has no association with any collision vertex (see Framework/DataTypes) |
Collaborator
There was a problem hiding this comment.
why not have the comments?
Comment on lines
-666
to
-684
| kIsDalitzLeg, // Up to 8 dalitz selections | ||
| kBarrelNAssocsInBunch = kIsDalitzLeg + 8, // number of in bunch collision associations | ||
| kBarrelNAssocsOutOfBunch, // number of out of bunch collision associations | ||
| kEMCalEnergy, // EMCal cluster energy | ||
| kEMCalCoreEnergy, // EMCal cluster core energy | ||
| kEMCalRawEnergy, // EMCal cluster raw energy | ||
| kEMCalEta, // EMCal cluster pseudorapidity | ||
| kEMCalPhi, // EMCal cluster azimuthal angle | ||
| kEMCalM02, // EMCal shower shape long axis | ||
| kEMCalM20, // EMCal shower shape short axis | ||
| kEMCalNCells, // number of cells in the EMCal cluster | ||
| kEMCalTime, // EMCal cluster time | ||
| kEMCalIsExotic, // exotic EMCal cluster flag | ||
| kEMCalDistanceToBadChannel, // distance of the EMCal cluster to the closest bad channel | ||
| kEMCalNLM, // number of local maxima of the EMCal cluster | ||
| kEMCalDefinition, // EMCal cluster definition | ||
| kEMCalEoverP, // EMCal cluster energy over matched track momentum | ||
| kEMCalMatchDeltaEta, // eta residual of the matched EMCal cluster w.r.t. the track | ||
| kEMCalMatchDeltaPhi, // phi residual of the matched EMCal cluster w.r.t. the track |
| values[kMuonDCAx] = static_cast<float>(dca[0]); | ||
| values[kMuonDCAy] = static_cast<float>(dca[1]); | ||
| values[kMuonDCAx] = static_cast<float>(dca[o2::dqconstants::defaults::IndexDCAxy]); | ||
| values[kMuonDCAy] = static_cast<float>(dca[o2::dqconstants::defaults::IndexDCAz]); |
Collaborator
There was a problem hiding this comment.
@axti98 You can leave this as you change it, but in my opinion this is too much. Same opinion for several other places which I think make the code much less readable.
@vkucera We might end up with so many of these constants that we will just keep increasing their number since it will be increasingly difficult to follow up which variables were defined already. To me this is not a good rule.
Comment on lines
+2661
to
+2662
| values[kIsSingleGap] = static_cast<float>(values[kIsSingleGapA] != o2::dqconstants::EmptyVariable || values[kIsSingleGapC] != o2::dqconstants::EmptyVariable); | ||
| values[kIsNoGap] = static_cast<float>(values[kIsDoubleGap] == o2::dqconstants::EmptyVariable && values[kIsSingleGap] == o2::dqconstants::EmptyVariable); |
Collaborator
There was a problem hiding this comment.
since we have "InvalidValueX", I would avoid naming this EmptyVariable. Its confusing i think. Please call it what it is: "o2::dqconstants::Zero"
Collaborator
|
@axti98 I suggest renaming this PR to something like "Fixing linter errors in VarManager" because refactoring would involve more structural changes, which is not the case here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactoring of the DQ Variable Manager to fix linter issues.
Introducing a DQConstants.h header to define constants that are known at compile time to improve code readability.