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

feat: bucket method generator sum (PROOF-642) #101

Merged
merged 3 commits into from
Feb 29, 2024
Merged

feat: bucket method generator sum (PROOF-642) #101

merged 3 commits into from
Feb 29, 2024

Conversation

rnburn
Copy link
Collaborator

@rnburn rnburn commented Feb 29, 2024

Rationale for this change

Add a function for the accumulation step of a version of the bucket method with many outputs.

What changes are included in this PR?

  • Adds a function to compute bucket sums.

Are these changes tested?

Yes.

// sum buckets
memmg::managed_array<T> sums_dev{num_buckets_total, &resource};
co_await std::move(fut);
basl::info("summing {} buckets", num_buckets_total);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would basl::info calls like this flood any info logs? I know we use this logging elsewhere in the code. Do you know if this logging impacts performance?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wouldn't expect it to impact performance much. You can turn off logging and cost of logging is small relative to the computation here.

@rnburn rnburn merged commit 826edc0 into main Feb 29, 2024
8 checks passed
@rnburn rnburn deleted the sm-PROOF-642 branch February 29, 2024 20:50
@SxT-Release
Copy link

🎉 This PR is included in version 1.37.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants