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

kp_kernel_logger.cpp: typo in end scan call back function #249

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

vlkale
Copy link
Contributor

@vlkale vlkale commented Apr 4, 2024

The end callback for scan in kp_kernel_logger.cpp has a typo in the function. This never broke kernel logger and didn't not cause a problem unless one used parallel_scan in their code. This causes a parent tool or tool utility to not recognize that this end callback exists when the tool utility or parent tool checks that its child library function call exists. This impacts the sampler utility and can also impact the kernel filter utility.

In any case, this one character edit to the code file needs to be fixed as soon as possible so that Kokkos user using Kokkos Tools kernel logger sees prints of parallel_scan in their kernel_logger output.

This is part of the original PR #213 but I am breaking that one down into small changes since there are a sizeable number and this impacts more than just the sampler.

@vlkale vlkale requested review from dalg24 and crtrott April 4, 2024 23:13
Copy link
Member

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

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

It would be a good time to add a unit test

@vlkale
Copy link
Contributor Author

vlkale commented Apr 4, 2024

It would be a good time to add a unit test

Yes, that makes sense. Do you mean for this PR? I can do that - not a problem.

By the way, there needs to be a (possibly large - in any case systematic) effort to develop tests for each tool connector. Maybe we do that in a new PR and create a corresponding Issue.

@vlkale
Copy link
Contributor Author

vlkale commented Apr 5, 2024

@dalg24 Actually, I see you approved this so I am guessing you mean put a unit test for the kernel-logger in another PR. Yes, I will do that. It will be specifically a test for the kernel-logger in the tests subdirectory of Kokkos Tools.

@dalg24 dalg24 merged commit 70b18b0 into kokkos:develop Apr 5, 2024
7 checks passed
@vlkale vlkale deleted the kernel-logger-end-scan-typo branch May 6, 2024 16:00
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.

3 participants