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

Caliper support for builtin Kokkos Tools (continued) #402

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

mzuzek
Copy link

@mzuzek mzuzek commented Dec 2, 2021

@ibaned @crtrott
This is continuation/extension of #397 by @DavidPoliakoff

Status

Example

See full example that connects Caliper to Kokkos via KokkosTools - using proposed EventSet-based approach:

int main(int argc, char *argv[]) {
  auto eventSet = KokkosTools::get_event_set("caliper",
                                             "runtime-report(profile.kokkos)");
  Kokkos::Tools::Experimental::set_callbacks(eventSet);

  Kokkos::initialize(argc, argv);
  // ... run app ... //
  Kokkos::finalize();
  return 0;
}

@mzuzek
Copy link
Author

mzuzek commented Dec 3, 2021

Should cali::get_event_set() be exposed in a Caliper header ?
If so, which one would be the best for that ?

@mzuzek mzuzek marked this pull request as ready for review December 3, 2021 09:54
@daboehme
Copy link
Member

daboehme commented Dec 7, 2021

Hi @MikolajZuzek , thanks for working on this.

Should cali::get_event_set() be exposed in a Caliper header ? If so, which one would be the best for that ?

If it only ever needs to be called from the Kokkos library we can probably just extern it. If it needs to be in a header it's probably best of in its own, like cali_kokkos.h.

For the functions themselves: I think it would be better to name them cali::get_kokkos_event_set and cali::set_kokkos_cali_config() to make it more explicit that they're for Kokkos.

@mzuzek mzuzek marked this pull request as draft February 22, 2022 23:55
@mzuzek
Copy link
Author

mzuzek commented Feb 22, 2022

Hi @daboehme!
Thank you for your answer and suggestions. My apologies for the delay on this reply.

I renamed the functions to what you've suggested, rebased this PR on current master and added some fixes for MSVC.

Let me convert this PR to draft for now: I guess it'll only make sense to merge it together or after kokkos/kokkos-tools#131.

@mzuzek mzuzek mentioned this pull request Feb 23, 2022
26 tasks
@daboehme
Copy link
Member

Let me convert this PR to draft for now: I guess it'll only make sense to merge it together or after kokkos/kokkos-tools#131.

Ok, sounds good. Let me know when this is ready to go in.

@vlkale
Copy link

vlkale commented May 24, 2024

@mzuzek cc. @daboehme I happened to come across this just now while looking at Caliper documentation, and I see that it was blocked at the time by the CMake build PR for Kokkos Tools. But, that CMake Build PR has been merged into Kokkos Tools develop branch. Are you able to finish and/or merge this PR now?

@mzuzek
Copy link
Author

mzuzek commented Jun 3, 2024

Hi @vlkale.

I've been reassigned to other things even before CMake build PR was merged into Kokkos Tools (kokkos/kokkos-tools#159) and, unfortunately, I'm not able to engage here at present...

PS. I guess you might first need Caliper team's insight on what would it take to get this PR finalized and merged.

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.

5 participants