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

Add precomputed sums for Unweighted + bug fix #13

Merged
merged 7 commits into from
Oct 18, 2022

Conversation

sfiligoi
Copy link
Collaborator

Use full emb sums when one side is fully zero in Unweighted.
Similar logic to what we have in Unweighted.

Also fix a memory access bug that could result in a core dump.

@sfiligoi
Copy link
Collaborator Author

CC @wasade

@sfiligoi
Copy link
Collaborator Author

The new logic adds a speedup on 1.8x on a 300k input using a GPU (not tested with CPU).
The speedup is more modest at 1.2x to 1.4x on a 50k input using either CPUs and GPUs.

Copy link
Member

@wasade wasade left a comment

Choose a reason for hiding this comment

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

Just a small question

src/unifrac_task.hpp Show resolved Hide resolved
@wasade
Copy link
Member

wasade commented Oct 17, 2022

The failure from the conda issue you had mentioned?

bool all_zeros=true;

#pragma acc loop seq
for (uint64_t emb=0; emb<filled_embs; emb++) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the bug in the previous PR.
Should have been filled_embs_els_round, as it is now in line 1181.

@sfiligoi sfiligoi merged commit 252f3a6 into biocore:main Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants