[PWGCF] Net charge #9525
[PWGCF] Net charge #9525
Conversation
nidamalikk
requested review from
alibuild,
saganatt,
victor-gonzalez,
zchochul,
lgraczykCern,
prchakra,
lauraser,
ariedel-cern,
otonvd and
shouqiye
as code owners
|
Any reason for not incorporating (merging) the suggested formatting changes? |
|
Please, rebase your changes to the latest version of |
| using namespace o2::framework::expressions; | ||
| using namespace std; | ||
|
|
||
| namespace o2::aod |
There was a problem hiding this comment.
Please, don't use this name space. Use your own if so you want
| { | ||
| histos.fill(HIST("hVertexZ_bef"), coll.posZ()); | ||
|
|
||
| if (fabs(coll.posZ()) > 10.f) { |
There was a problem hiding this comment.
Precede it with std::, i.e. std::fabs
| histos.fill(HIST("hCentrality"), coll.centRun2V0M()); | ||
| histos.fill(HIST("hMultiplicity"), coll.multFV0M()); | ||
|
|
||
| for (auto track : inputTracks) { |
There was a problem hiding this comment.
Use a const reference qualifier, if not your full track is copied here which is not efficient
| histos.fill(HIST("hMultiplicity"), coll.multFV0M()); | ||
|
|
||
| for (auto track : inputTracks) { | ||
| if (fabs(track.eta()) > 0.8) |
There was a problem hiding this comment.
Incorporate std::
| @@ -43,3 +43,8 @@ o2physics_add_dpl_workflow(factorial-moments | |||
| SOURCES FactorialMomentsTask.cxx | |||
| PUBLIC_LINK_LIBRARIES O2::Framework O2Physics::AnalysisCore O2Physics::PWGCFCore | |||
| COMPONENT_NAME Analysis) | |||
|
|
|||
| o2physics_add_dpl_workflow(netchargefluct | |||
There was a problem hiding this comment.
You will need to use netcharge-fluctuations according to the name of your source file and you will need to change the name of this one to netchargeFluctuations.cxx
| using MyTrack = MyTracks::iterator; | ||
| } // namespace o2::aod | ||
|
|
||
| struct netchargeFluctuations { |
There was a problem hiding this comment.
You have to keep the first letter as capital, NetchargeFluctuations
There was a problem hiding this comment.
Okay. My file name is netchargeFluctuations.cxx. Would you like me to change its first letter also?
There was a problem hiding this comment.
No, just the struct name. If you don't do it now you will be requested to do it latter by the linter
You already changed the file name to netchargeFluctuations.cxx which is correct according to the name you selected for your workflow in the CMakeLists.txt file
| histos.fill(HIST("hCentrality"), coll.centRun2V0M()); | ||
| histos.fill(HIST("hMultiplicity"), coll.multFV0M()); | ||
|
|
||
| for (const auto track : inputTracks) { |
There was a problem hiding this comment.
You still need to fully qualify it as constant reference, if not the full track is copied here which is quite inefficient
There was a problem hiding this comment.
This was the first time I created a pull request. Please tell me what change I should make to fully qualify it as constant reference.
There was a problem hiding this comment.
You have to use for (auto const& track : inputTracks)
The type is indicated by the auto, which will be resolved by the compiler. The reference to that type is indicated by the &. And the fact that what the reference refers cannot be changed, a compiler error will happen if it is, is indicated by the const preceding the reference indication
There was a problem hiding this comment.
Thank you for helping me out.
There was a problem hiding this comment.
There are still two things you need to do
- remove the old
NetchargeFluctuations.cxxfile - rebase your changes to the latest version of O2Physics. The latest version of O2Physics has not been used for implementing these changes and that's why the message This branch has conflicts that must be resolved is shown
There was a problem hiding this comment.
The linter recommendations that you can see in the CMakeLists.txt file are not yours, they are from other analyzers and they will take care of them in a timely manner
But, please, follow the linter recommendations on your source file, netchargeFluctuations.cxx, in the next iteration
|
Dear @nidamalikk and @victor-gonzalez, this commit removed my workflow from the |
@SwatiSaha-1997 this is usually taken care by the own gitHub machinery So I guess something happened with gitHub which considered resolved a conflict which was not and that ended up in a We always enforce to rebase the changes to the latest version of O2Physics but when commits come closer to each other, even gitHub gets confused |
This code generates the QA plots for net charge fluctuation study by using an observable nu_dyn.