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

Update pgo_nsys_converter.py NVTX kern sum report name #22972

Conversation

mgoldfarb-nvidia
Copy link
Contributor

@mgoldfarb-nvidia mgoldfarb-nvidia commented Aug 9, 2024

Report name has changed with latest nsys.

If older versions are installed pgo_nsys_converter.py will fall back to the old report name.

Copy link

google-cla bot commented Aug 9, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@mgoldfarb-nvidia mgoldfarb-nvidia deleted the mgoldfarb-nvidia/pgo_nsys_converter_update branch August 9, 2024 18:52
@mgoldfarb-nvidia mgoldfarb-nvidia restored the mgoldfarb-nvidia/pgo_nsys_converter_update branch August 22, 2024 13:51
@mgoldfarb-nvidia mgoldfarb-nvidia force-pushed the mgoldfarb-nvidia/pgo_nsys_converter_update branch 2 times, most recently from f4517ba to f9dba24 Compare August 23, 2024 13:18
@mgoldfarb-nvidia mgoldfarb-nvidia changed the title Update pgo_nsys_converter.py path for NVTX kern sum report Update pgo_nsys_converter.py NVTX kern sum report name Aug 23, 2024
@shraiysh
Copy link

@mgoldfarb-nvidia can you please change this to get the nvtx_sum report instead of the nvtx_kern_sum? I am not sure what this option was called before nsys 2023.1, (maybe nvtxsum?). Can you also find it out and handle that like you handled both nvtx_kern_sum and nvtxkernsum?

We need this because memory operations are not included in the protobuf file - they should be. We have an incomplete profile because we are using the wrong report.

@mgoldfarb-nvidia mgoldfarb-nvidia force-pushed the mgoldfarb-nvidia/pgo_nsys_converter_update branch from f9dba24 to d2b1ebd Compare August 26, 2024 17:29
@mgoldfarb-nvidia
Copy link
Contributor Author

@mgoldfarb-nvidia can you please change this to get the nvtx_sum report instead of the nvtx_kern_sum? I am not sure what this option was called before nsys 2023.1, (maybe nvtxsum?). Can you also find it out and handle that like you handled both nvtx_kern_sum and nvtxkernsum?

We need this because memory operations are not included in the protobuf file - they should be. We have an incomplete profile because we are using the wrong report.

Updated using nvtx_sum and nvtxsum for older nsys releases.

@shraiysh
Copy link

How do we add reviewers to this PR @mgoldfarb-nvidia?

@mgoldfarb-nvidia
Copy link
Contributor Author

How do we add reviewers to this PR @mgoldfarb-nvidia?
@nouiz might be able to help get this some eyes from the maintainers.

@google-ml-butler google-ml-butler bot added kokoro:force-run pull ready Ready for copybara import and testing labels Aug 27, 2024
@copybara-service copybara-service bot merged commit 88a2008 into jax-ml:main Aug 27, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull ready Ready for copybara import and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants