Femtoworld#1011
Conversation
|
Can you remove redundant AliFemto and O2Femto directories? |
|
You need to rebase your changes to the latest tag |
victor-gonzalez
left a comment
victor-gonzalez
left a comment
There was a problem hiding this comment.
Please, organize your code with the same directory layout as the PWGCF/MultiparticleCorrelations directory
Core for your internal classes definition
DataModel for your internal columns/tables definitions
TableProducer for your tasks which generate internal tables to be consumed by other task
Tasks for your analysis tasks
|
@victor-gonzalez we copied the FemtoDream so we organized the code like them. Will later all CF frameworks have the same structure? We will of course conform in this case :) |
|
@victor-gonzalez this is still a part of our learning of O2. As @saganatt wrote, our code for the moment is a copy of the FemtoDream that we will start adjusting to our needs for Phi and D meson analysis, but first, we need to test what we have for the moment and also learn how to use Hyperloop. |
|
I understand it but it is better to start with the right structure from the beginning than correct it later |
|
@saganatt yes, all CF frameworks have to have the same structure |
|
OK @victor-gonzalez, it's fine; we will rearrange the structure to match what you propose. Indeed it is true it will make some order in the files and you're right, better do it from the beginning. |
| # | ||
| # In applying this license CERN does not waive the privileges and immunities | ||
| # granted to it by virtue of its status as an Intergovernmental Organization | ||
| # or submit itself to any jurisdiction. |
There was a problem hiding this comment.
Have in mind that you don't necessarily need to only have headers files in your Core directory.
You could also have .cxx files when needed and in that case you should instruct the compiler to create a library that you will link with your table producer tasks or with your task as needed
Check https://github.com/AliceO2Group/O2Physics/blob/master/PWGCF/Core/CMakeLists.txt for inspiration
| #ifndef ANALYSIS_TASKS_PWGCF_FEMTOWORLD_FEMTOWORDLCONTAINER_H_ | ||
| #define ANALYSIS_TASKS_PWGCF_FEMTOWORLD_FEMTOWORDLCONTAINER_H_ |
There was a problem hiding this comment.
Probably it does make sense to get rid of the ANALYSIS_TASKS_PWGCF_FEMTOWORLD_ head
What do you think?
| #ifndef ANALYSIS_TASKS_PWGCF_O2FEMTO_O2FEMTODETADPHISTAR_H_ | ||
| #define ANALYSIS_TASKS_PWGCF_O2FEMTO_O2FEMTODETADPHISTAR_H_ |
There was a problem hiding this comment.
The same as above and removing additionally the O2
| #ifndef ANALYSIS_TASKS_PWGCF_FEMTOWORLD_INCLUDE_FEMTOWORLD_FEMTOEVENTHISTO_H_ | ||
| #define ANALYSIS_TASKS_PWGCF_FEMTOWORLD_INCLUDE_FEMTOWORLD_FEMTOEVENTHISTO_H_ |
There was a problem hiding this comment.
The same but in this case with ANALYSIS_TASKS_PWGCF_FEMTOWORLD_INCLUDE_FEMTOWORLD_
| #ifndef ANALYSIS_TASKS_PWGCF_O2FEMTO_O2FEMTOMATH_H_ | ||
| #define ANALYSIS_TASKS_PWGCF_O2FEMTO_O2FEMTOMATH_H_ |
There was a problem hiding this comment.
Same as above but with ANALYSIS_TASKS_PWGCF_O2FEMTO_O2
| #ifndef ANALYSIS_TASKS_PWGCF_FEMTOWORLD_INCLUDE_FEMTOWORLD_FEMTOWORLDPAIRCLEANER_H_ | ||
| #define ANALYSIS_TASKS_PWGCF_FEMTOWORLD_INCLUDE_FEMTOWORLD_FEMTOWORLDPAIRCLEANER_H_ |
There was a problem hiding this comment.
The same as above but with ANALYSIS_TASKS_PWGCF_FEMTOWORLD_INCLUDE_FEMTOWORLD_
| #ifndef ANALYSIS_TASKS_PWGCF_FEMTOWORLD_INCLUDE_FEMTOWORLD_FEMTOPARTICLEHISTO_H_ | ||
| #define ANALYSIS_TASKS_PWGCF_FEMTOWORLD_INCLUDE_FEMTOWORLD_FEMTOPARTICLEHISTO_H_ |
There was a problem hiding this comment.
The same as above but with ANALYSIS_TASKS_PWGCF_FEMTOWORLD_INCLUDE_FEMTOWORLD_
| #ifndef ANALYSIS_TASKS_PWGCF_FEMTOWORLD_UTILS_H_ | ||
| #define ANALYSIS_TASKS_PWGCF_FEMTOWORLD_UTILS_H_ |
There was a problem hiding this comment.
The same as above but with ANALYSIS_TASKS_PWGCF_
| float getMagneticFieldTesla(const uint64_t& timestamp, const Service<o2::ccdb::BasicCCDBManager>& ccdb) | ||
| { | ||
| // TODO done only once (and not per run). Will be replaced by CCDBConfigurable | ||
| static o2::parameters::GRPObject* grpo = ccdb->getForTimeStamp<o2::parameters::GRPObject>("GLO/GRP/GRP", timestamp); |
There was a problem hiding this comment.
Have in mind that for Run 3 the CCDB endpoint and the way of getting the magnetic field information has changed
Have a look at the implementation of PR#976 https://github.com/AliceO2Group/O2Physics/pull/976/files for inspiration
If you are not addressing Run 3 datasets for the time being you don't need to consider it. Just have it in mind
saganatt
left a comment
saganatt
left a comment
There was a problem hiding this comment.
Hi @victor-gonzalez thank you very much for your comments!
We will indeed improve the ifndef guard clauses, they got a bit messy. And we will look at the CCDB later.
Since it doesn't affect the code performance now, I hope you doesn't mind merging this as-it, to be available on Hyperloop tomorrow? And tomorrow Zuzanna will fix the header files.
|
It does make sense to me! |
* FemtoWorld_v.21.07 * Femtoworld changes part 1 * FemtoWorld new folders
4 participants
No description provided.