Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update repo: committing code updates for Lc->pKpi analysis #255

Merged
merged 25 commits into from
Jan 11, 2022

Conversation

strogolo
Copy link
Contributor

Dear @fcatalan92, @fgrosa,
I am doing this PR in order to push in the repository the updates that I have done for the non-prompt Lc analysis in pp collisions at 13 TeV.
In particular I have updated:

  • GetRawYieldsDplusDs.py
  • ProjectDplusDsSparse.py
  • ProjectDplusDsTree.py
  • RunFullAnalysis.sh
    and I have committed as new macro/files:
  • ComputeEffAccWeightedAvg.py
  • ComputeEffAccWeightedAvg.C
  • config_LctopKpi_Fit_pp13TeV_basic.yml

Please let me know if something is not correct and has to be fixed.
Thanks and regards,
Stefano

Copy link
Member

@fcatalan92 fcatalan92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @strogolo, thank you for your contribution. I added a few comments.
Moreover, I would prefer to have only one macro for the weighted average of the efficiency. I would keep ComputeEffAccWeightedAvg.py, we have still a few macros that are both in C++ and in Python but it is not a good idea.

Comment on lines +22 to +26
gROOT.SetBatch(args.batch)
SetGlobalStyle(padleftmargin=0.14, padbottommargin=0.12, titlesize=0.045, labelsize=0.04)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? You are not drawing anything in the macro.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the previous version of the script I was not drawing the histograms, while now I'm doing it in the latest update

Comment on lines 25 to 53
effFileAll = TFile.Open(args.effFileAll)
hEffAllPrompt = effFileAll.Get('hAccEffPrompt')
hEffAllFD = effFileAll.Get('hAccEffFD')
SetObjectStyle(hEffAllPrompt, color=kRed+1, markerstyle=kFullCircle)
SetObjectStyle(hEffAllFD, color=kAzure+4, markerstyle=kOpenSquare, markersize=1.5, linewidh=2, linestyle=7)

effFileNonRes = TFile.Open(args.effFileNonRes)
hEffNonResPrompt = effFileNonRes.Get('hAccEffPrompt')
hEffNonResFD = effFileNonRes.Get('hAccEffFD')
SetObjectStyle(hEffNonResPrompt, color=kRed+1, markerstyle=kFullCircle)
SetObjectStyle(hEffNonResFD, color=kAzure+4, markerstyle=kOpenSquare, markersize=1.5, linewidh=2, linestyle=7)

effFileKStar = TFile.Open(args.effFileKStar)
hEffKStarPrompt = effFileKStar.Get('hAccEffPrompt')
hEffKStarFD = effFileKStar.Get('hAccEffFD')
SetObjectStyle(hEffKStarPrompt, color=kRed+1, markerstyle=kFullCircle)
SetObjectStyle(hEffKStarFD, color=kAzure+4, markerstyle=kOpenSquare, markersize=1.5, linewidh=2, linestyle=7)

effFileDelta = TFile.Open(args.effFileDelta)
hEffDeltaPrompt = effFileDelta.Get('hAccEffPrompt')
hEffDeltaFD = effFileDelta.Get('hAccEffFD')
SetObjectStyle(hEffDeltaPrompt, color=kRed+1, markerstyle=kFullCircle)
SetObjectStyle(hEffDeltaFD, color=kAzure+4, markerstyle=kOpenSquare, markersize=1.5, linewidh=2, linestyle=7)

effFileLambda1520 = TFile.Open(args.effFileLambda1520)
hEffLambda1520Prompt = effFileLambda1520.Get('hAccEffPrompt')
hEffLambda1520FD = effFileLambda1520.Get('hAccEffFD')
SetObjectStyle(hEffLambda1520Prompt, color=kRed+1, markerstyle=kFullCircle)
SetObjectStyle(hEffLambda1520FD, color=kAzure+4, markerstyle=kOpenSquare, markersize=1.5, linewidh=2, linestyle=7)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would do a for loop since you are copy-pasting the same operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-wrote everything using loops in python. It took me sometime to get everything working since I am not super-expert of python. Nevertheless, I checked the output and the result is identical to the old version of the script.

Comment on lines 28 to 29
SetObjectStyle(hEffAllPrompt, color=kRed+1, markerstyle=kFullCircle)
SetObjectStyle(hEffAllFD, color=kAzure+4, markerstyle=kOpenSquare, markersize=1.5, linewidh=2, linestyle=7)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting the style for these histograms looks not needed since you are not drawing/storing them

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is needed in the last version of the script in order to draw the histograms.

GetRawYieldsDplusDs.C Show resolved Hide resolved
GetRawYieldsDplusDs.C Show resolved Hide resolved
Comment on lines 158 to 165
if args.LctopKpireso == 1:
dataFramePrompt = FilterBitDf(dataFramePrompt, 'cand_type', [bitLcNonRes], 'and')
if args.LctopKpireso == 2:
dataFramePrompt = FilterBitDf(dataFramePrompt, 'cand_type', [bitLcKStar], 'and')
if args.LctopKpireso == 3:
dataFramePrompt = FilterBitDf(dataFramePrompt, 'cand_type', [bitLcDelta], 'and')
if args.LctopKpireso == 4:
dataFramePrompt = FilterBitDf(dataFramePrompt, 'cand_type', [bitLcLambda1520], 'and')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can rewrite this by making a list of the different bits and accessing the list with args.LctopKpireso to avoid the copy-paste

Suggested change
if args.LctopKpireso == 1:
dataFramePrompt = FilterBitDf(dataFramePrompt, 'cand_type', [bitLcNonRes], 'and')
if args.LctopKpireso == 2:
dataFramePrompt = FilterBitDf(dataFramePrompt, 'cand_type', [bitLcKStar], 'and')
if args.LctopKpireso == 3:
dataFramePrompt = FilterBitDf(dataFramePrompt, 'cand_type', [bitLcDelta], 'and')
if args.LctopKpireso == 4:
dataFramePrompt = FilterBitDf(dataFramePrompt, 'cand_type', [bitLcLambda1520], 'and')
if args.LctopKpireso in range(1, 5):
dataFramePrompt = FilterBitDf(dataFramePrompt, 'cand_type', [bitsLcResonance[args.LctopKpireso]], 'and')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 173 to 180
if args.LctopKpireso == 1:
dataFrameFD = FilterBitDf(dataFrameFD, 'cand_type', [bitLcNonRes], 'and')
if args.LctopKpireso == 2:
dataFrameFD = FilterBitDf(dataFrameFD, 'cand_type', [bitLcKStar], 'and')
if args.LctopKpireso == 3:
dataFrameFD = FilterBitDf(dataFrameFD, 'cand_type', [bitLcDelta], 'and')
if args.LctopKpireso == 4:
dataFrameFD = FilterBitDf(dataFrameFD, 'cand_type', [bitLcLambda1520], 'and')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if args.LctopKpireso == 1:
dataFrameFD = FilterBitDf(dataFrameFD, 'cand_type', [bitLcNonRes], 'and')
if args.LctopKpireso == 2:
dataFrameFD = FilterBitDf(dataFrameFD, 'cand_type', [bitLcKStar], 'and')
if args.LctopKpireso == 3:
dataFrameFD = FilterBitDf(dataFrameFD, 'cand_type', [bitLcDelta], 'and')
if args.LctopKpireso == 4:
dataFrameFD = FilterBitDf(dataFrameFD, 'cand_type', [bitLcLambda1520], 'and')
if args.LctopKpireso in range(1, 5):
dataFramePrompt = FilterBitDf(dataFramePrompt, 'cand_type', [bitsLcResonance[args.LctopKpireso]], and')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines +209 to +215
if [ ${Particle} == "LctopKpi" ]; then
python3 ${ProjectScript} ${cfgFileMC} ${CutSetsDir}/cutset${CutSets[$iCutSet]}.yml ${OutDirEfficiency}/Distr_${Particle}_MC${CutSets[$iCutSet]}.root
python3 ${ProjectScript} ${cfgFileMCNonRes} ${CutSetsDir}/cutset${CutSets[$iCutSet]}.yml ${OutDirEfficiency}/Distr_${Particle}_NonRes_MC${CutSets[$iCutSet]}.root --LctopKpireso 1
python3 ${ProjectScript} ${cfgFileMCKStar} ${CutSetsDir}/cutset${CutSets[$iCutSet]}.yml ${OutDirEfficiency}/Distr_${Particle}_KStar_MC${CutSets[$iCutSet]}.root --LctopKpireso 2
python3 ${ProjectScript} ${cfgFileMCDelta} ${CutSetsDir}/cutset${CutSets[$iCutSet]}.yml ${OutDirEfficiency}/Distr_${Particle}_Delta_MC${CutSets[$iCutSet]}.root --LctopKpireso 3
python3 ${ProjectScript} ${cfgFileMCLambda1520} ${CutSetsDir}/cutset${CutSets[$iCutSet]}.yml ${OutDirEfficiency}/Distr_${Particle}_Lambda1520_MC${CutSets[$iCutSet]}.root --LctopKpireso 4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you are removing the possibility to apply pT weights and other corrections for the Lc->pkpi. Maybe we need to think about how to improve the approach and build the command incrementally

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will implement the possiblity to apply pT weights when I'll have to do it.

Comment on lines 260 to 270
if [ ${Particle} == "LctopKpi" ]; then
echo $(tput setaf 4) Compute efficiency from ${OutDirEfficiency}/Distr_${Particle}_MC${CutSets[$iCutSet]}.root $(tput sgr0)
python3 ComputeEfficiencyDplusDs.py ${cfgFileFit} ${Cent} ${OutDirEfficiency}/Distr_${Particle}_MC${CutSets[$iCutSet]}.root ${OutDirEfficiency}/Efficiency_${Particle}${CutSets[$iCutSet]}.root --batch
echo $(tput setaf 4) Compute efficiency from ${OutDirEfficiency}/Distr_${Particle}_NonRes_MC${CutSets[$iCutSet]}.root $(tput sgr0)
python3 ComputeEfficiencyDplusDs.py ${cfgFileFit} ${Cent} ${OutDirEfficiency}/Distr_${Particle}_NonRes_MC${CutSets[$iCutSet]}.root ${OutDirEfficiency}/Efficiency_${Particle}_NonRes${CutSets[$iCutSet]}.root --batch
echo $(tput setaf 4) Compute efficiency from ${OutDirEfficiency}/Distr_${Particle}_KStar_MC${CutSets[$iCutSet]}.root $(tput sgr0)
python3 ComputeEfficiencyDplusDs.py ${cfgFileFit} ${Cent} ${OutDirEfficiency}/Distr_${Particle}_KStar_MC${CutSets[$iCutSet]}.root ${OutDirEfficiency}/Efficiency_${Particle}_KStar${CutSets[$iCutSet]}.root --batch
echo $(tput setaf 4) Compute efficiency from ${OutDirEfficiency}/Distr_${Particle}_Delta_MC${CutSets[$iCutSet]}.root $(tput sgr0)
python3 ComputeEfficiencyDplusDs.py ${cfgFileFit} ${Cent} ${OutDirEfficiency}/Distr_${Particle}_Delta_MC${CutSets[$iCutSet]}.root ${OutDirEfficiency}/Efficiency_${Particle}_Delta${CutSets[$iCutSet]}.root --batch
echo $(tput setaf 4) Compute efficiency from ${OutDirEfficiency}/Distr_${Particle}_Lambda1520_MC${CutSets[$iCutSet]}.root $(tput sgr0)
python3 ComputeEfficiencyDplusDs.py ${cfgFileFit} ${Cent} ${OutDirEfficiency}/Distr_${Particle}_Lambda1520_MC${CutSets[$iCutSet]}.root ${OutDirEfficiency}/Efficiency_${Particle}_Lambda1520${CutSets[$iCutSet]}.root --batch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here a for loop could be used

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 284 to 288
python3 CombineAccTimesEff.py ${OutDirEfficiency}/Efficiency_${Particle}${CutSets[$iCutSet]}.root ${accFileName} ${OutDirEfficiency}/Eff_times_Acc_${Particle}${CutSets[$iCutSet]}.root --batch
python3 CombineAccTimesEff.py ${OutDirEfficiency}/Efficiency_${Particle}_NonRes${CutSets[$iCutSet]}.root ${accFileName} ${OutDirEfficiency}/Eff_times_Acc_${Particle}_NonRes${CutSets[$iCutSet]}.root --batch
python3 CombineAccTimesEff.py ${OutDirEfficiency}/Efficiency_${Particle}_KStar${CutSets[$iCutSet]}.root ${accFileName} ${OutDirEfficiency}/Eff_times_Acc_${Particle}_KStar${CutSets[$iCutSet]}.root --batch
python3 CombineAccTimesEff.py ${OutDirEfficiency}/Efficiency_${Particle}_Delta${CutSets[$iCutSet]}.root ${accFileName} ${OutDirEfficiency}/Eff_times_Acc_${Particle}_Delta${CutSets[$iCutSet]}.root --batch
python3 CombineAccTimesEff.py ${OutDirEfficiency}/Efficiency_${Particle}_Lambda1520${CutSets[$iCutSet]}.root ${accFileName} ${OutDirEfficiency}/Eff_times_Acc_${Particle}_Lambda1520${CutSets[$iCutSet]}.root --batch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe for loop also here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

RunFullAnalysis.sh Outdated Show resolved Hide resolved
@strogolo
Copy link
Contributor Author

strogolo commented Jan 5, 2022

Hi @strogolo, thank you for your contribution. I added a few comments. Moreover, I would prefer to have only one macro for the weighted average of the efficiency. I would keep ComputeEffAccWeightedAvg.py, we have still a few macros that are both in C++ and in Python but it is not a good idea.

Hi @fcatalan92, I have implemented almost all of your suggestions. Regarding the double version of the macro ComputeEffAccWeightedAvg (in python and in C++) I understand your point. However, I just want to inform you that on my laptop I'm using the C++ version, while the python version was written for completeness for your repository. Thus, I was just wondering if it is good to have at least a version of the macro that I'm currently using as documentation.

Thanks and regards,
Stefano

ComputeEffAccWeightedAvg.py Outdated Show resolved Hide resolved
ComputeEffAccWeightedAvg.py Outdated Show resolved Hide resolved
ComputeEffAccWeightedAvg.py Outdated Show resolved Hide resolved
ComputeEffAccWeightedAvg.py Outdated Show resolved Hide resolved
ComputeEffAccWeightedAvg.py Outdated Show resolved Hide resolved
ComputeEffAccWeightedAvg.py Outdated Show resolved Hide resolved
@fcatalan92
Copy link
Member

Hi @strogolo, thank you for implementing the changes. I have just a few suggestions more.

On the ComputeEffAccWeightedAvg, have you checked that the python version is equivalent to the C++ one (same outputs)? If yes, I think we can merge only the former one. @fgrosa What do you think?

@strogolo
Copy link
Contributor Author

Hi @strogolo, thank you for implementing the changes. I have just a few suggestions more.

On the ComputeEffAccWeightedAvg, have you checked that the python version is equivalent to the C++ one (same outputs)? If yes, I think we can merge only the former one. @fgrosa What do you think?

Hi @fcatalan92, I have removed the debug print as requested.
Concerning your question: I did not check in details that the output of the python version is equivalent to the C++ one. From a quick look it seems everything equivalent, but I did not perform an apple-to-apple comparison because the main macro I'm using for the analysis is the C++ one, while I wrote the python for completeness and because someone could prefer to use the python version. Look, if it is such a huge problem I can remove ComputeEffAccWeightedAvg (both python and C++) since it is a macro specific for the Lc->pKpi analysis and just keep it locally updated.

Copy link
Member

@fgrosa fgrosa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ciao @strogolo thanks a lot! I have a couple of comments that just go in the direction to simplify a bit the code (if they are fine with you you can just click on "commit suggestion"). For the pT weights, indeed it will have to be done, but it's ok for me to do it in a second step (also because in the meantime we might want to merge this PR #249). For the c++/python script I have no strong opinion, it is fine for me to keep only one of the two or both.

ComputeEffAccWeightedAvg.C Outdated Show resolved Hide resolved
ComputeEffAccWeightedAvg.C Outdated Show resolved Hide resolved
ComputeEffAccWeightedAvg.C Outdated Show resolved Hide resolved
ComputeEffAccWeightedAvg.C Outdated Show resolved Hide resolved
ComputeEffAccWeightedAvg.C Outdated Show resolved Hide resolved
ComputeEffAccWeightedAvg.py Outdated Show resolved Hide resolved
ComputeEffAccWeightedAvg.py Outdated Show resolved Hide resolved
ComputeEffAccWeightedAvg.py Outdated Show resolved Hide resolved
ComputeEffAccWeightedAvg.py Outdated Show resolved Hide resolved
RunFullAnalysis.sh Outdated Show resolved Hide resolved
@fgrosa fgrosa mentioned this pull request Jan 10, 2022
@fcatalan92
Copy link
Member

@strogolo Thank you for the updates. I made just a minor comment.
IMHO, the most important thing is to have only one macro to maintain. So, if it is not possible to validate the python one (my preferred option), I would keep only the C++ macro.

strogolo and others added 7 commits January 11, 2022 10:18
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
strogolo and others added 7 commits January 11, 2022 10:24
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
Co-authored-by: Fabrizio <fabrizio.grosa@cern.ch>
@strogolo
Copy link
Contributor Author

@strogolo Thank you for the updates. I made just a minor comment. IMHO, the most important thing is to have only one macro to maintain. So, if it is not possible to validate the python one (my preferred option), I would keep only the C++ macro.

Hi @fcatalan92 thanks for your reply. What I can propose is that we merge both macros and in the next weeks, when I'll do the systematic studies, I'll try to use the python script to check that works properly. Once this is verified, we can remove the C++ macro from the repository. Could this be a viable option? In this way we can go on with this PR (without blocking others) and in parallel have the time to do some checks. Thanks, Stefano

ComputeEffAccWeightedAvg.py Outdated Show resolved Hide resolved
Copy link
Member

@fcatalan92 fcatalan92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@strogolo fine for me to merge now and remove the C++ macro after the checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants