PWGCF: fixed QA flags#7962
Merged
Merged
Conversation
abilandz
requested review from
alibuild,
saganatt,
victor-gonzalez and
zchochul
as code owners
victor-gonzalez
requested changes
Oct 11, 2024
victor-gonzalez
left a comment
victor-gonzalez
left a comment
Collaborator
There was a problem hiding this comment.
Please, check that you are not rolling back changes done for standardization and converting compiler warnings to errors
I pointed them out for you to check them
Also
- there was an issue with the
absfunction which started to return anintas it should. But this caused a lot of unexpected behaviors because people had extensively used it. Check that you are usingstd::absinstead - Prefix your PR and commit titles with
PWGCF:
|
|
||
| // C) Process only simulated data: | ||
| void processSim(CollisionSim const& /*collision*/, aod::BCs const&, TracksSim const& /*tracks*/) | ||
| void processSim(CollisionSim const& collision, aod::BCs const&, TracksSim const& tracks) |
Collaborator
There was a problem hiding this comment.
Please, don't remove the commented out collisions and tracks
We are in the process of converting all warnings to errors and this is a not used variable warning
They were changed in the past directly on gitHub so probably you are still having locally an old version
Collaborator
Author
There was a problem hiding this comment.
This was rolled back by mistake - I pushed a new commit, which brings this back!
|
|
||
| // F) Process only converted simulated Run 2 data: | ||
| void processSim_Run2(CollisionSim const& /*collision*/) // TBI 20240517 extend this subscription eventually | ||
| void processSim_Run2(CollisionSim const& collision) // TBI 20240517 extend this subscription eventually |
|
|
||
| // H) Process both converted reconstructed and corresponding MC truth simulated Run 1 data; | ||
| void processRecSim_Run1(CollisionRecSim_Run1 const& /*collision*/, aod::BCs const&, TracksRecSim const& /*tracks*/, aod::McParticles const&, aod::McCollisions const&) | ||
| void processRecSim_Run1(CollisionRecSim_Run1 const& collision, aod::BCs const&, TracksRecSim const& tracks, aod::McParticles const&, aod::McCollisions const&) |
| pc.fPtDependentDCAxyFormula = new TFormula("fPtDependentDCAxyFormula", pc.fsParticleCuts[ePtDependentDCAxyParameterization].Data()); | ||
| // As a quick insanity check, try immediately to evaluate something from this formula: | ||
| if (std::isnan(pc.fPtDependentDCAxyFormula->Eval(1.44))) { | ||
| if (isnan(pc.fPtDependentDCAxyFormula->Eval(1.44))) { |
Collaborator
There was a problem hiding this comment.
Any reason for removing the std:: on isnan?
I think this was introduced directly on gitHub to standardize the code and perhaps you are still using an old local version. Please, check it
Collaborator
Author
There was a problem hiding this comment.
I didn't pull these changes in my class done directly on GitHub, that's the only reason. In the latest commit, I switched back to std::
| // *) std::isnan() check (remember that 'nan' is 0./0., inf-inf, etc. However 'inf' itself is NOT a 'nan', therefore std::isnan(1./0.) is false, std::isnan(0./0.) is true, etc.): | ||
| if (std::isnan(track.phi()) || std::isnan(track.pt()) || std::isnan(track.eta())) { | ||
| // *) isnan() check (remember that 'nan' is 0./0., inf-inf, etc. However 'inf' itself is NOT a 'nan', therefore isnan(1./0.) is false, isnan(0./0.) is true, etc.): | ||
| if (isnan(track.phi()) || isnan(track.pt()) || isnan(track.eta())) { |
victor-gonzalez
approved these changes
Oct 12, 2024
Collaborator
|
Thanks! |
mfaggin
pushed a commit
to mfaggin/O2Physics
that referenced
this pull request
Oct 15, 2024
* fixed QA flags * fix for formatter 1 * pulling back central updates
louisemillot
pushed a commit
to louisemillot/O2Physics
that referenced
this pull request
Nov 26, 2024
* fixed QA flags * fix for formatter 1 * pulling back central updates
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
2 participants
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.
No description provided.