Skip to content

added check plots and modified binning #1059

Closed
rosarioturrisi wants to merge 4 commits into
AliceO2Group:masterfrom
rosarioturrisi:master
Closed

added check plots and modified binning #1059
rosarioturrisi wants to merge 4 commits into
AliceO2Group:masterfrom
rosarioturrisi:master

Conversation

@rosarioturrisi

Copy link
Copy Markdown
Contributor

No description provided.

@mfaggin mfaggin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you Rosario, just a few comments to be sure I understood your changes. Thank you.

Comment thread DPG/Tasks/qaMatchEff.cxx
// non uniform pt binning
std::vector<double> ptBinning = {0.0, 0.05, 0.1, 0.15, 0.2, 0.25, 0.3, 0.35, 0.4, 0.45, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0};
//
AxisSpec axisPDG{pdgBins, 0, pdgBins + 1.000, "pdgclass"};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is there a reason for a double here? Can't it be an integer like pdgBins + 1?

Comment thread DPG/Tasks/qaMatchEff.cxx
itr_pdg = std::find(pdgChoice.begin(), pdgChoice.end(), tpPDGCode);
if (itr_pdg != pdgChoice.cend())
// index from zero, so increase by 1 to put in the right bin (and 0.5 not needed but just not to sit in the edge)
pdg_fill = (float)std::distance(pdgChoice.begin(), itr_pdg) + 1.5;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

confirm me if I get properly: here the logic is that you want to keep 0 for unmatched tracks (i.e. fakes) and then above 1 a species per bin, depenging on how many of the pdgChoice one you decide to keep at runtime with pdgBins. Am I right?

Comment thread DPG/Tasks/qaMatchEff.cxx
} // end pions only
//
// no primary/sec-d pions
if (!((tpPDGCode == 211) && (mcpart.isPhysicalPrimary()))) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

here you are requiring that the particle is a Physical Primary, but not a pion. Is it the combination you want?

Comment thread DPG/Tasks/qaMatchEff.cxx
// index from zero, so increase by 1 to put in the right bin (and 0.5 not needed but just not to sit in the edge)
pdg_fill = (float)std::distance(pdgChoice.begin(), itr_pdg) + 1.5;
else
pdg_fill = -10.0;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This -10.0 goes in the underflow, so if I understand properly:

  • with 0 you keep flag fake tracks
  • with -10 those with a MC particle that is not in the bunch of those you decided to check.
    Is it correct?

@rosarioturrisi

rosarioturrisi commented Jul 26, 2022 via email

Copy link
Copy Markdown
Contributor Author

@alibuild

Copy link
Copy Markdown
Collaborator

Error while checking build/O2Physics/o2 for 36a1f7e at 2022-07-27 04:51:

## sw/BUILD/O2Physics-latest/log
c++: fatal error: Killed signal terminated program cc1plus
ninja: build stopped: subcommand failed.

Full log here.

@mfaggin mfaggin enabled auto-merge (squash) July 27, 2022 06:33
@mfaggin

mfaggin commented Jul 27, 2022

Copy link
Copy Markdown
Collaborator

thanks @rosarioturrisi for the clarifications

@mfaggin mfaggin disabled auto-merge July 27, 2022 08:51
@mfaggin mfaggin self-requested a review July 27, 2022 08:51
@mfaggin mfaggin enabled auto-merge (squash) July 27, 2022 08:52
@mfaggin

mfaggin commented Jul 27, 2022

Copy link
Copy Markdown
Collaborator

@TimoWilken I do not know if you are the right person to solve the issue we have here, but in case can you have a look or redirect it to the right person? Thank you.

@mfaggin

mfaggin commented Jul 27, 2022

Copy link
Copy Markdown
Collaborator

@TimoWilken I do not know if you are the right person to solve the issue we have here, but in case can you have a look or redirect it to the right person? Thank you.

tagging also @jgrosseo for completeness

@mfaggin mfaggin closed this Jul 27, 2022
auto-merge was automatically disabled July 27, 2022 12:17

Pull request was closed

@mfaggin

mfaggin commented Jul 27, 2022

Copy link
Copy Markdown
Collaborator

changes addressed in PR #1063

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