Skip to content

Conversation

@cwpearson
Copy link
Contributor

@cwpearson cwpearson commented Jan 16, 2025

We had a user who wasn't getting the nvtx connector built as expected. This may help clarify what's going on in the future.

Signed-off-by: Carl Pearson <cwpears@sandia.gov>
@cwpearson
Copy link
Contributor Author

cwpearson commented Jan 16, 2025

#281 should fix the simple build failure.

Signed-off-by: Carl Pearson <cwpears@sandia.gov>
@cwpearson cwpearson requested a review from dalg24 January 17, 2025 16:22
Copy link
Contributor

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

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

Would you mind adding similar messages for all the other tools?

@vlkale
Copy link
Contributor

vlkale commented Jan 17, 2025

Would you mind adding similar messages for all the other tools?

I assume you mean for all the other third-party library connector tools in Kokkos Tools? It doesn't make sense for the in-house tools in Kokkos Tools, e.g., SpaceTimeStack, right?

For this PR, maybe we do just VTune connector, roctx connector, and nvtx connector. I think handling a tool like the APEX Kokkos Tools connector may be slightly more complicated.

I see that the roctx connector is handled now too in this PR.

@masterleinad
Copy link
Contributor

The ones I noticed missing are VTune and Variorum since they also don't have a CMake option. It might be a good idea to just have a CMake option for everything but that can clearly be a different pull request.

@vlkale
Copy link
Contributor

vlkale commented Jan 17, 2025

The ones I noticed missing are VTune and Variorum since they also don't have a CMake option. It might be a good idea to just have a CMake option for everything but that can clearly be a different pull request.

Got it, that makes sense then - and I think for this PR, it should just be ensured the CMake logic for each of the connectors consistent with each other, as @dalg24 commented.

Thanks @cwpearson for looking at this!

add_subdirectory(profiling/nvtx-connector)
add_subdirectory(profiling/nvtx-focused-connector)
else()
message(STATUS "Skipping nvtx-connector: NVTX is not enabled (maybe Kokkos was not found, or not configured with the CUDA backend)")
Copy link
Member

Choose a reason for hiding this comment

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

I feel like that the message is potentially confusing, I would do just

Suggested change
message(STATUS "Skipping nvtx-connector: NVTX is not enabled (maybe Kokkos was not found, or not configured with the CUDA backend)")
message(STATUS "Skipping nvtx-connector (NVTX disabled)")

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.

4 participants