Repository navigation
[PWGDQ] Add new process function and fix variables in VarManager - #18694
Merged
Merged
Conversation
ThePhDane
requested review from
XiaozhiBai,
alibuild,
axti98,
glegras,
iarsene,
lucamicheletti93,
mguilbau and
shreyasiacharya
as code owners
November 11, 2026 14:55
iarsene
reviewed
Nov 11, 2026
Comment on lines
+917
to
+919
| //Magnus, Variable definition of U2Q2 for FT0A and FT0C | ||
| kU2Q2FT0A, | ||
| kU2Q2FT0C, |
Collaborator
There was a problem hiding this comment.
remove the comment line and add comment to the right of each of the two variables
| using MyEventsVtxCovGlobalMultSelected = o2::soa::Join<o2::aod::ReducedEvents, o2::aod::ReducedEventsExtended, o2::aod::ReducedEventsVtxCov, o2::aod::ReducedEventsGlobalTrk, o2::aod::EventCuts>; | ||
| using MyEventsVtxCovSelected = o2::soa::Join<o2::aod::ReducedEvents, o2::aod::ReducedEventsExtended, o2::aod::ReducedEventsVtxCov, o2::aod::EventCuts>; | ||
| using MyEventsVtxCovSelectedInfo = o2::soa::Join<o2::aod::ReducedEvents, o2::aod::ReducedEventsExtended, o2::aod::ReducedEventsVtxCov, o2::aod::EventCuts, o2::aod::ReducedEventsInfo>; | ||
| // Magnus, Declaration of name for event table with vertex covariance and Qvector information. |
| using MyMuonTracksWithCovWithAmbiguities = o2::soa::Join<o2::aod::ReducedMuons, o2::aod::ReducedMuonsExtra, o2::aod::ReducedMuonsCov, o2::aod::MuonAmbiguities>; | ||
| using MyMuonTracksSelectedWithColl = o2::soa::Join<o2::aod::ReducedMuons, o2::aod::ReducedMuonsExtra, o2::aod::ReducedMuonsInfo, o2::aod::MuonTrackCuts>; | ||
|
|
||
| // Magnus, Added a new fillmap for event with Qvector and centrality. |
| { | ||
| fEnableBarrelHistos = context.mOptions.get<bool>("processAllSkimmed") || context.mOptions.get<bool>("processBarrelOnlySkimmed") || context.mOptions.get<bool>("processBarrelOnlyWithCollSkimmed") || context.mOptions.get<bool>("processBarrelOnlySkimmedNoCov") || context.mOptions.get<bool>("processBarrelOnlySkimmedNoCovWithMultExtra") || context.mOptions.get<bool>("processBarrelOnlyWithQvectorCentrSkimmedNoCov") || context.mOptions.get<bool>("processBarrelOnlyWithQvectorCentrSkimmed") || context.mOptions.get<bool>("processBarrelOnlyDiKaons") || context.mOptions.get<bool>("processBarrelOnlyDiProtons") || context.mOptions.get<bool>("processBarrelOnlySkimmedWithGlobalMult"); | ||
| fEnableBarrelMixingHistos = context.mOptions.get<bool>("processMixingAllSkimmed") || context.mOptions.get<bool>("processMixingBarrelSkimmed") || context.mOptions.get<bool>("processMixingBarrelSkimmedFlow") || context.mOptions.get<bool>("processMixingBarrelWithQvectorCentrSkimmedNoCov") || context.mOptions.get<bool>("processMixingBarrelSkimmedGlobalMult"); | ||
| // Magnus, Added new process function to enabled histograms. |
Removed comments indicating the addition of new fill maps and process functions related to Qvector and vertex covariance.
|
O2 linter results: ❌ 0 errors, |
Collaborator
|
Error while checking build/O2Physics/staging for b0b30a1 at 2026-11-11 17:15: Full log here. |
Collaborator
|
Error while checking build/O2Physics/o2 for baa4e62 at 2026-11-11 17:15: Full log here. |
Collaborator
|
Error while checking build/O2Physics/code-check for baa4e62 at 2026-11-11 17:18: Full log here. |
Collaborator
|
Error while checking build/O2Physics/staging for baa4e62 at 2026-11-11 18:03: Full log here. |
Collaborator
|
Error while checking build/O2Physics/code-check for 95a6f42 at 2026-11-12 06:21: Full log here. |
Added NOLINT comment to suppress reinterpret cast warning of line 834 fTrackCuts.push_back(reinterpret_cast<AnalysisCompositeCut*>(t)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast)
vkucera
reviewed
Nov 12, 2026
| std::vector<AnalysisCut*> addTrackCuts = o2::aod::dqcuts::GetCutsFromJSON(addTrackCutsStr.Data()); | ||
| for (auto const& t : addTrackCuts) { | ||
| fTrackCuts.push_back(reinterpret_cast<AnalysisCompositeCut*>(t)); | ||
| fTrackCuts.push_back(reinterpret_cast<AnalysisCompositeCut*>(t)); // NOLINT(cppcoreguidelines-pro-type-reinterpret-cast) |
Collaborator
There was a problem hiding this comment.
Why are you suppressing the error?
Contributor
Author
There was a problem hiding this comment.
I just needed to make sure that the error was for this and not some of my changes. Do you have any suggestions on how to fix the issue?
Collaborator
There was a problem hiding this comment.
Hi, if I may add: Using reinterpret_cast is definitely the wrong approach here and should be removed, however, for now, it produces the desired behaviour. It was changed to dynamic_cast in some other PR, but the problem is that this cast returns nullptr if the t is a plain AnalysisCut object. This can lead to the fTrackCuts vector to be empty.
We have a solution in mind to avoid that cast entirely, but we have to test this. So for now I would kindly ask to have the reinterpret_cast still in the code, we will publish a fix soon.
Contributor
Author
There was a problem hiding this comment.
Okay. I will remove the suppression then, and then I will let it be up to you if/how the error is to be resolved.
Collaborator
Collaborator
There was a problem hiding this comment.
Hi, the dynamic_cast was producing a crash at run time, so I replaced with the reinterpret_cast, which gives the appropriate behaviour.
Collaborator
There was a problem hiding this comment.
We will fix this. IN principle we don't need any cast here, but only when we actually use the objects
Collaborator
There was a problem hiding this comment.
i will merge this now because this is more general, but i think I can find a fix since I anyway have a test at hand from my previous recent PR
Collaborator
There was a problem hiding this comment.
the problem is that this cast returns
nullptrif thetis a plainAnalysisCutobject.
That is the intended behaviour and the only correct behaviour. That is the very reason why dynamic_cast should be used here.
reinterpret_cast... for now, it produces the desired behaviour.
No, it does not! It does a silent invalid downcast which is an undefined behaviour. This is not a workaround, this is simply a bug!
Hi, the dynamic_cast was producing a crash at run time, so I replaced with the reinterpret_cast, which gives the appropriate behaviour.
It does not give the appropriate behaviour. It is just hiding a bug.
Removed the suppression of an unrelated error. This is a known issue which is being worked on
Collaborator
|
Error while checking build/O2Physics/code-check for 73127b0 at 2026-11-12 10:13: Full log here. |
iarsene
approved these changes
Nov 12, 2026
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.
I have added a new process function, which includes Qvector, centrality and Vertex covariance. Additionally I have fixed the definitions of pseudoproper decay lenght, so it has the appropriate units, and I have added U2Q2 values for FT0A and FT0C