Skip to content

Conversation

@wujingyue
Copy link
Collaborator

@wujingyue wujingyue commented Nov 3, 2025

Instead, check whether the script is under nsys via NSYS_PROFILING_SESSION_ID. Note that it's still possible to profile warmup iterations -- just don't specify --capture-range cudaProfilerStart in the nsys command.

This partially rolls back #2661.

Instead, check whether the script is under nsys via
`NSYS_PROFILE_SESSION_ID`. Note that it's still possible to profile
warmup iterations -- just don't specify `--capture-range
cudaProfilerStart` in the `nsys` command.
@wujingyue wujingyue requested review from crcrpar, kshitij12345 and mattteochen and removed request for kshitij12345 November 3, 2025 19:24
Copy link
Collaborator

@mattteochen mattteochen left a comment

Choose a reason for hiding this comment

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

Looks good, thank you.

@wujingyue
Copy link
Collaborator Author

I checked the CI errors and they are not related to this PR. Ready to merge!

@wujingyue wujingyue enabled auto-merge (squash) November 4, 2025 18:04
@kshitij12345
Copy link
Collaborator

I wanted to read more about NSYS_PROFILE_SESSION_ID but couldn't find the documentation at https://docs.nvidia.com/nsight-systems/search.html?q=NSYS_PROFILE_SESSION_ID. Can you please share the link? Thanks!

@wujingyue
Copy link
Collaborator Author

I wanted to read more about NSYS_PROFILE_SESSION_ID but couldn't find the documentation at https://docs.nvidia.com/nsight-systems/search.html?q=NSYS_PROFILE_SESSION_ID. Can you please share the link? Thanks!

It's not official and that's a valid concern. I think it's OK because in the worst case we see an empty nsys stats. But I don't mind to drop the PR.

PS: it should be NSYS_PROFILING_SESSION_ID instead. However, that still isn't mentioned in any official documentation.

cc @kshitij12345

Copy link
Collaborator

@kshitij12345 kshitij12345 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @wujingyue

Copy link
Collaborator

@t-vi t-vi left a comment

Choose a reason for hiding this comment

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

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