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

PWGHF: new task for W/Z->e in mid-rapidity #7582

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

sashingo
Copy link

@sashingo sashingo commented Sep 5, 2024

PWGHF: a new task for W/Z decay to electron in midrapidity in Run3

PWGHF/HFL/Tasks/CMakeLists.txt Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/CMakeLists.txt Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
@sashingo sashingo closed this Sep 6, 2024
@sashingo sashingo reopened this Sep 6, 2024
@sashingo
Copy link
Author

sashingo commented Sep 6, 2024

Dear Vit,
thank you very much for reviewing the commit.

PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
@sashingo
Copy link
Author

sashingo commented Sep 9, 2024

recommitted after implementing suggestions by Vit

@vkucera
Copy link
Collaborator

vkucera commented Sep 10, 2024

Thanks @sashingo for implementing the comments.
Please try to stick to the O2 naming conventions and use the lowerCamelCase style for the names of variables and functions. It improves readability of your code and helps others understand it.

PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
PWGHF/HFL/Tasks/taskElectronWeakBoson.cxx Outdated Show resolved Hide resolved
@sashingo
Copy link
Author

Hello, I think I address all the comments by Vit.
Please check and approve if it's ok.

NicoleBastid
NicoleBastid previously approved these changes Sep 25, 2024
@NicoleBastid NicoleBastid enabled auto-merge (squash) September 26, 2024 07:05
auto-merge was automatically disabled September 30, 2024 08:25

Head branch was pushed to by a user without write access

Copy link
Collaborator

@vkucera vkucera left a comment

Choose a reason for hiding this comment

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

Hi @sashingo , thanks for implementing the comments. Please seem some further suggestions.

Comment on lines 210 to 214
if (dPhi > o2::constants::math::PI) {
dPhi -= 2 * o2::constants::math::PI;
} else if (dPhi < -o2::constants::math::PI) {
dPhi += 2 * o2::constants::math::PI;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (dPhi > o2::constants::math::PI) {
dPhi -= 2 * o2::constants::math::PI;
} else if (dPhi < -o2::constants::math::PI) {
dPhi += 2 * o2::constants::math::PI;
}
dPhi = RecoDecay::constrainAngle(dPhi, -o2::constants::math::PI);

Requires including RecoDecay.h.

registry.fill(HIST("hMatchPhi"), phiEmc, match.track_as<TrackEle>().phi());
registry.fill(HIST("hMatchEta"), etaEmc, match.track_as<TrackEle>().eta());

double R = std::sqrt(std::pow(dPhi, 2) + std::pow(dEta, 2));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
double R = std::sqrt(std::pow(dPhi, 2) + std::pow(dEta, 2));
double r = RecoDecay::sqrtSumOfSquares(dPhi, dEta);

Comment on lines 221 to 223
Rmim = R;
dPhi_mim = dPhi;
dEta_mim = dEta;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Rmim = R;
dPhi_mim = dPhi;
dEta_mim = dEta;
rMin = r;
dPhiMin = dPhi;
dEtaMin = dEta;

double dEta_mim = 999.9;

if (tracksofcluster.size()) {
int nmatch = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
int nmatch = 0;
int nMatch = 0;

Comment on lines 257 to 258
return WorkflowSpec{
adaptAnalysisTask<HfTaskElectronWeakBoson>(cfgc)};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return WorkflowSpec{
adaptAnalysisTask<HfTaskElectronWeakBoson>(cfgc)};
return WorkflowSpec{adaptAnalysisTask<HfTaskElectronWeakBoson>(cfgc)};

@sashingo
Copy link
Author

sashingo commented Oct 1, 2024

Hi Vit,
thank you for further suggestions.
I implemented them

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

Successfully merging this pull request may close these issues.

3 participants