Skip to content

PWGHF: Unify different partitions in Ds task#8035

Merged
mfaggin merged 5 commits into
AliceO2Group:masterfrom
fchinu:fix_n_contrib
Oct 22, 2024
Merged

PWGHF: Unify different partitions in Ds task#8035
mfaggin merged 5 commits into
AliceO2Group:masterfrom
fchinu:fix_n_contrib

Conversation

@fchinu
Copy link
Copy Markdown
Contributor

@fchinu fchinu commented Oct 17, 2024

This PR fixes the filling of the number of contributors in the Ds task output.
After an interesting chat with @aalkin, I was suggested to drop the usage of the std::map to indicate the partition to be used, as it increases both compilation and run time, and remove redundant multiple definitions of the partitions, as it is not needed to have different specialisations for the different Joins.

Please consider the following formatting changes to AliceO2Group#8035
@fchinu fchinu changed the title PWGHF: use contain<T> to choose the partition to use instead of an std::map in Ds task PWGHF: use contains<T> to choose the partition to use instead of an std::map in Ds task Oct 17, 2024
fgrosa
fgrosa previously approved these changes Oct 17, 2024
Copy link
Copy Markdown
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.

Nice solution, thanks!

@fgrosa fgrosa enabled auto-merge (squash) October 17, 2024 12:18
auto-merge was automatically disabled October 18, 2024 10:01

Head branch was pushed to by a user without write access

@github-actions github-actions Bot added the pwghf PWG-HF label Oct 18, 2024
@aalkin
Copy link
Copy Markdown
Member

aalkin commented Oct 19, 2024

@fchinu I took a closer look at the code, and what you are doing with selecting preslice is in fact redundant. Since you are always slicing on the MC collision index, there is no reason to have several instances of preslice, it is actually enough to have PresliceUnsorted<CollisionsMc> colPerMcCollision = aod::mccollisionlabel::mcCollisionId; or even PresliceUnsorted<aod::McCollisionLabels> colPerMcCollision = aod::mccollisionlabel::mcCollisionId; and it is usable for any join that contains it.

@fchinu fchinu changed the title PWGHF: use contains<T> to choose the partition to use instead of an std::map in Ds task PWGHF: Unify different partitions in Ds task Oct 21, 2024
@fchinu
Copy link
Copy Markdown
Contributor Author

fchinu commented Oct 21, 2024

Thanks a lot @aalkin for your time and suggestions! I have changed the code following your advice, it is now much cleaner!

Copy link
Copy Markdown
Collaborator

@mfaggin mfaggin left a comment

Choose a reason for hiding this comment

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

all good for me, I approve it. @aalkin if you can confirm that it's ok for you as well I merge it. Thanks

@aalkin
Copy link
Copy Markdown
Member

aalkin commented Oct 22, 2024

The code looks good, please go ahead and merge it.

@mfaggin mfaggin merged commit 09161d6 into AliceO2Group:master Oct 22, 2024
@fchinu fchinu deleted the fix_n_contrib branch October 23, 2024 07:12
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.

5 participants