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] Extend information of sparses for efficiency evaluation. Refactor analysis utilities. #9153

Merged
merged 14 commits into from
Jan 11, 2025

Conversation

Marcellocosti
Copy link
Contributor

@Marcellocosti Marcellocosti commented Dec 29, 2024

In this PR, occupancy and centrality axes are added to existing sparses. Plus, the information on the B hadron mother pdg and pt is stored for nonprompt candidates. Finally, sparse histograms are added for generated MC candidates.

Refactor utility methods and enumerators:

  • getCentralityColl
  • getCentralityGenColl
  • getBHadMotherFlag
  • getOccupancyColl
  • getOccupancyGenColl
  • BHadMothers
  • OccupancyEstimator

@vkucera
Copy link
Collaborator

vkucera commented Jan 6, 2025

There is quite some duplication with #9142. It would be best to factor the common code out into utility headers, especially the enumerators and the occupancy helper functions.

@Marcellocosti
Copy link
Contributor Author

Hi @vkucera, @fgrosa, I modified the PR according to your suggestion to reduce code duplication. First, I moved the getBHadMotherFlag, used by both the Ds and D+ tasks, in the utilsAnalysis.h. Then, I created a namespace o2::hf_occupancy placed in the utilsHfEvSel.h file for the occupancy functions. Plus, I moved the centrality estimation function in its respective header, deleting the other similar functions present in taskDs.cxx, taskDplus.cxx, taskFlowCharmHadrons, taskMcValidation and utilsHfEvSel.h. The previous functions present in the header cover only the cases where only one centrality column is joined with the particle candidates table. The functions I implemented should cover the cases where a task-specific set of columns is present in the particle candidate table. Please let me know if this solution works for you, tagging also @stefanopolitano as you may be interested.

Copy link
Collaborator

@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.

Hi @Marcellocosti it looks very good to me, thanks! It can be merged in my opinion, let's see if it looks good to @vkucera as well.

/// \param collSlice collection of reconstructed collisions associated to a generated one
/// \return generated MC collision occupancy
template <typename CCs>
int getOccupancyGenColl(CCs const& collSlice, int occEstimator = 1)
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 getOccupancyGenColl(CCs const& collSlice, int occEstimator = 1)
int getOccupancyGenColl(CCs const& collSlice, int occEstimator = OccupancyEstimator::Its)

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.

Thanks for the nice refactorisation. Please see my suggestions for improvements and safety fixes.

/// \param centEstimator integer to select the centrality estimator
/// \return collision centrality
template <typename Coll>
float getCentralityColl(const Coll& collision, int centEstimator)
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
float getCentralityColl(const Coll& collision, int centEstimator)
float getCentralityColl(const Coll& collision, CentralityEstimator centEstimator)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The most common use-case of this function is when the centEstimator argument is a Configurable<int> type defined in the user tasks. As far as I am aware, casting is not defined from Configurable<int> types to enum types, so I don't know how to implement this conversion. Do you have any suggestion?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Use centEstimator.value as argument of calling getCentralityColl.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understood correctly, centEstimator is a Configurable<int> type, centEstimator.value extracts an int value. But then I manage to get a working version only by passing to the function static_cast<o2::hf_centrality::CentralityEstimator>(centEstimator.value) since the casting from the int to an enum type is not automatic.

PWGHF/Core/CentralityEstimation.h Outdated Show resolved Hide resolved
PWGHF/Core/CentralityEstimation.h Outdated Show resolved Hide resolved
PWGHF/Core/CentralityEstimation.h Outdated Show resolved Hide resolved
PWGHF/Utils/utilsAnalysis.h Outdated Show resolved Hide resolved
PWGHF/Utils/utilsEvSelHf.h Outdated Show resolved Hide resolved
PWGHF/Utils/utilsEvSelHf.h Outdated Show resolved Hide resolved
@Marcellocosti
Copy link
Contributor Author

Dear @vkucera, I have implemented your suggestions apart from the conversion of the int to its respective enum type for the estimator argument in the getters as I get compiler errors since the cast is not automatic.

@fgrosa
Copy link
Collaborator

fgrosa commented Jan 10, 2025

Hi @vkucera could you please check if the latest version is ok for you and possibly approve it? This PR is urgent for QM approvals. Thanks!

@vkucera vkucera changed the title [PWGHF] Extend information of sparses for efficiency evaluation [PWGHF] Extend information of sparses for efficiency evaluation. Refactor analysis utilities. Jan 11, 2025
@vkucera vkucera merged commit 80fae3e into AliceO2Group:master Jan 11, 2025
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pwghf PWG-HF
Development

Successfully merging this pull request may close these issues.

4 participants