PWGHF: Add single particle distribution

[njacazio]

• Extendend configurability of task
• Add possibility to set the PDG code of the particle of interest

[njacazio]

Hi @vkucera thanks for the useful comments, indeed it was quite some refactoring but now the situation should be considerably improved

[njacazio]

Closed by mistake, ok I'll see

[ginnocen]

hi @njacazio can you have a quick look at this PR, @vkucera has a few comments on it.
Thanks!

[njacazio]

Indeed! I will have a look after after tomorrow

[ginnocen]

hi @njacazio @vkucera what should we do about this PR?

[vkucera]

@njacazio Any updates?

[njacazio]

Hi, no news, I performed the tests at the time for debug of the simulation but I would say that this is not needed anymore. I would close this and then I will bring it up if needed again.

[vkucera]

Hi, no news, I performed the tests at the time for debug of the simulation but I would say that this is not needed anymore. I would close this and then I will bring it up if needed again.

What was the point of reviewing and updating this PR several times then???
By the way, there are bugs in the current code that were supposed to be fixed with this PR.

[njacazio]

Hi, it's time to revive this, I also added few additional plots

[njacazio]

I also realized I missed your last comment, sorry!

[vkucera]

Capitalization Rules

Variables and functions start with a lowercase letter.
Everything else in C++ code (type names, constant expressions) starts with an uppercase letter.

[njacazio]

Hi Vit, what you don't like with the capitalization?

[vkucera]

Hi Vit, what you don't like with the capitalization?

The struct names: qaGlobalObservables, qaTrackingKine, qaTrackingResolution don't follow the quoted rules.

[njacazio]

Ok, I should have taken care of all the comments, I'm moving this as ready to be reviewed to start tests. Thanks a lot @vkucera

[ginnocen]

hi @njacazio @vkucera what is the destiny of this task supposed to be :)?

[njacazio]

Hi @ginnocen to be merged hopefully

[vkucera]

@njacazio It seems you missed my request from your other PR.
Please don't amend the reviewed commits in the PR. Push a new commit on top of the existing ones. It becomes less transparent when new changes get merged with the original commits and replace what has been reviewed.

[njacazio]

@vkucera no I did not miss it, with latest commit I fixed a CI error and in principle all comments have been implemented. @ginnocen can we make something for the merging?

[njacazio]

@vkucera re-ready for re-review!

[njacazio]

@vkucera the output of the space checker is a bit cryptic to interpret to say the least, do you have any hint at what it refers to?
Also, could it be that it refers to parts of the code that I did not touch?

[njacazio]

@vkucera follow up on the "space checker", I think it is checking a file version which has nothing to do with this PR and that are in the main dev I guess we can take care of those later in another PR. If you could quickly approve the changes we can go forth and have it finally merged

[njacazio]

Hi @vkucera by any chance, do you have any other suggestion ? Do you have any news on the space checker failing? Did you have a look at the log?

[vkucera]

@njacazio Approving but for the next PR, please:

• Do not overwrite already reviewed commits.
• Explicitly specify namespace qualifier (e.g. std::sqrt instead of sqrt).
• Consider renaming the file qaTask.cxx to something that includes event, track to make it less ambiguous and better matching the name of the workflow.
• Improve readability by using a variable for a repeatedly used value instead of calling it every time to avoid code like this:

    histograms.fill(HIST("collision/X"), collision.posX());
    histograms.fill(HIST("collision/Y"), collision.posY());
    histograms.fill(HIST("collision/Z"), collision.posZ());

    histograms.fill(HIST("collision/XvsNContrib"), collision.posX(), collision.numContrib());
    histograms.fill(HIST("collision/YvsNContrib"), collision.posY(), collision.numContrib());
    histograms.fill(HIST("collision/ZvsNContrib"), collision.posZ(), collision.numContrib());

• Using more explicit names for label variables, such as in:

const TString eta = "#it{#eta}";

where eta is not pseudorapidity but an axis label.

[vkucera]

@ginnocen @jgrosseo @iarsene The failing CI tests seem to be caused by too high RAM consumption in compilation.

[njacazio]

@ginnocen the CI failing seem to be spurious, if this is the case could you merge the PR?

[njacazio]

Hi @ginnocen we are already ready to merge the PR, could you please take care of it?