Uh oh!
There was an error while loading. Please reload this page.
PWGCF: upload PbPb flow analysis for hyperloop - #4088
Conversation
victor-gonzalez
left a comment
There was a problem hiding this comment.
Thanks for moving forward the first Flow GF analysis in O2!
Please, consider my comments before approval
Uh oh!
There was an error while loading. Please reload this page.
| if (SampleIndex == 0) | ||
| FillBootstrap<0>(cent, ptSum_Gap08, weffEvent_WithinGap08, sum_pt_wSquare_WithinGap08, sum_ptSquare_wSquare_WithinGap08, WeffEvent_diff_WithGap08); | ||
| else if (SampleIndex == 1) | ||
| FillBootstrap<1>(cent, ptSum_Gap08, weffEvent_WithinGap08, sum_pt_wSquare_WithinGap08, sum_ptSquare_wSquare_WithinGap08, WeffEvent_diff_WithGap08); | ||
| else if (SampleIndex == 2) | ||
| FillBootstrap<2>(cent, ptSum_Gap08, weffEvent_WithinGap08, sum_pt_wSquare_WithinGap08, sum_ptSquare_wSquare_WithinGap08, WeffEvent_diff_WithGap08); | ||
| else if (SampleIndex == 3) | ||
| FillBootstrap<3>(cent, ptSum_Gap08, weffEvent_WithinGap08, sum_pt_wSquare_WithinGap08, sum_ptSquare_wSquare_WithinGap08, WeffEvent_diff_WithGap08); | ||
| else if (SampleIndex == 4) | ||
| FillBootstrap<4>(cent, ptSum_Gap08, weffEvent_WithinGap08, sum_pt_wSquare_WithinGap08, sum_ptSquare_wSquare_WithinGap08, WeffEvent_diff_WithGap08); | ||
| else if (SampleIndex == 5) | ||
| FillBootstrap<5>(cent, ptSum_Gap08, weffEvent_WithinGap08, sum_pt_wSquare_WithinGap08, sum_ptSquare_wSquare_WithinGap08, WeffEvent_diff_WithGap08); | ||
| else if (SampleIndex == 6) | ||
| FillBootstrap<6>(cent, ptSum_Gap08, weffEvent_WithinGap08, sum_pt_wSquare_WithinGap08, sum_ptSquare_wSquare_WithinGap08, WeffEvent_diff_WithGap08); | ||
| else if (SampleIndex == 7) | ||
| FillBootstrap<7>(cent, ptSum_Gap08, weffEvent_WithinGap08, sum_pt_wSquare_WithinGap08, sum_ptSquare_wSquare_WithinGap08, WeffEvent_diff_WithGap08); | ||
| else if (SampleIndex == 8) | ||
| FillBootstrap<8>(cent, ptSum_Gap08, weffEvent_WithinGap08, sum_pt_wSquare_WithinGap08, sum_ptSquare_wSquare_WithinGap08, WeffEvent_diff_WithGap08); | ||
| else if (SampleIndex == 9) | ||
| FillBootstrap<9>(cent, ptSum_Gap08, weffEvent_WithinGap08, sum_pt_wSquare_WithinGap08, sum_ptSquare_wSquare_WithinGap08, WeffEvent_diff_WithGap08); |
There was a problem hiding this comment.
Couldn't it be enough to pass the SampleIndex as the template parameter and avoid the chain of ifs?
There was a problem hiding this comment.
I also do not like this chain of if-else, but the HIST("") is a static function and do not accept variables. It's welcomed if you have any other idea.
There was a problem hiding this comment.
You could receive the histogram pointers and put them in an array?
There was a problem hiding this comment.
Hi Jan, thanks for your suggestions, I also find it hard to receive pointers due to the subdirectory:
- static HIST("") make it hard to loop the subsamples (Number of subsamples may change in configurable)
- I also try to loop over the TDirectory, but static_cast<TList*> will break the subdirectory structure
For my view, it's better to store the pointers in a TList object at the beginning. As it's late at night in Asia, I will make the changes tomorrow
There was a problem hiding this comment.
Hi victor, Jan,
I have implemented the changes. Please take a look.
There was a problem hiding this comment.
Hi victor, Jan, I have implemented the changes. Please take a look.
Sorry, I am traveling today and only on the phone.
I believe registry.add returns the pointer.
Also I was wondering if it makes sense to use the index as a histogram axis instead by adding one dimension (and using THn)
There was a problem hiding this comment.
Hi victor, Jan, I have implemented the changes. Please take a look.
Sorry, I am traveling today and only on the phone.
I believe registry.add returns the pointer.
Also I was wondering if it makes sense to use the index as a histogram axis instead by adding one dimension (and using THn)
Hi Jan,
Yes I can simplify my code using the return pointers from registry.add(). I will do this later.
For my sight, using the index of histograms or using a TProfile2D are the same. But maybe it's due to my lack of experience and I don't know the benefits to use TProfile2D. If you have any comment on this, please tell me.
Uh oh!
There was an error while loading. Please reload this page.
| BootstrapArray[i][0] = new TProfile(Form("hMeanPtWithinGap08_%d", i), Form("hMeanPtWithinGap08_%d", i), nMultiBins, &multiBins[0]); | ||
| BootstrapArray[i][1] = new TProfile(Form("c22_gap08_Weff_%d", i), Form("c22_gap08_Weff_%d", i), nMultiBins, &multiBins[0]); | ||
| BootstrapArray[i][2] = new TProfile(Form("c22_gap08_trackMeanPt_%d", i), Form("c22_gap08_trackMeanPt_%d", i), nMultiBins, &multiBins[0]); | ||
| BootstrapArray[i][3] = new TProfile(Form("PtVariance_partA_WithinGap08_%d", i), Form("PtVariance_partA_WithinGap08_%d", i), nMultiBins, &multiBins[0]); | ||
| BootstrapArray[i][4] = new TProfile(Form("PtVariance_partB_WithinGap08_%d", i), Form("PtVariance_partB_WithinGap08_%d", i), nMultiBins, &multiBins[0]); | ||
| for (int j = 0; j < 5; j++) { | ||
| BootstrapArray[i][j]->Sumw2(); | ||
| fOutputList->Add(BootstrapArray[i][j]); | ||
| } | ||
| } |
There was a problem hiding this comment.
I would keep the sub-directories structure that you had previously by using instead registry.add which, as Jan Fiete said, should return the TProfile pointer which you can store in your array
I would suggest also to, instead of using 0, 1, 2, 3, 4, to select your TProfile within a concrete sub-sample, to use an enum as, for example,
enum theProfiles {
kMeanPt = 0,
kC22Weff,
kC22MeanPt,
kPtVarParA,
kPtVarParB
};
or something similar, which will allow you to remember at each time what you are filling.
There was a problem hiding this comment.
sure, I will apply it in next PR
| if (weffEvent_WithinGap08 > 1e-6) | ||
| BootstrapArray[SampleIndex][0]->Fill(cent, ptSum_Gap08 / weffEvent_WithinGap08, weffEvent_WithinGap08); | ||
| if (weffEvent_WithinGap08 > 1e-6) | ||
| FillpTvnProfile(corrconfigs.at(7), ptSum_Gap08, weffEvent_WithinGap08, BootstrapArray[SampleIndex][1], BootstrapArray[SampleIndex][2], cent); | ||
| if (WeffEvent_diff_WithGap08 > 1e-6) { | ||
| BootstrapArray[SampleIndex][3]->Fill(cent, | ||
| (ptSum_Gap08 * ptSum_Gap08 - sum_ptSquare_wSquare_WithinGap08) / WeffEvent_diff_WithGap08, | ||
| WeffEvent_diff_WithGap08); | ||
| BootstrapArray[SampleIndex][4]->Fill(cent, | ||
| (weffEvent_WithinGap08 * ptSum_Gap08 - sum_pt_wSquare_WithinGap08) / WeffEvent_diff_WithGap08, | ||
| WeffEvent_diff_WithGap08); | ||
| } |
There was a problem hiding this comment.
If you decide to use the above suggested enum use them here as indexes of your profiles in the corresponding sub-sample
* upload PbPb flow analysis for hyperloop * format source file with git-clang-format * fix the error informed by MegaLinter * add a new line at the end of file according to MegaLinter * remove commented lines * Use TList to store Bootstrap samples * move <vector> up according to MegaLinter * simplify code with return pointers of registry.add
* upload PbPb flow analysis for hyperloop * format source file with git-clang-format * fix the error informed by MegaLinter * add a new line at the end of file according to MegaLinter * remove commented lines * Use TList to store Bootstrap samples * move <vector> up according to MegaLinter * simplify code with return pointers of registry.add
the code to produce flow observables for PbPb run3 data