Skip to content

[rocpd] Use SQL queries instead of views in summary generator#311

Merged
yhuiYH merged 8 commits into
developfrom
atumakae/summary_sql_queries
Sep 15, 2025
Merged

[rocpd] Use SQL queries instead of views in summary generator#311
yhuiYH merged 8 commits into
developfrom
atumakae/summary_sql_queries

Conversation

@amd-atumakae

@amd-atumakae amd-atumakae commented Aug 12, 2025

Copy link
Copy Markdown
Contributor

PR Details

Associated Jira Ticket Number/Link

SWDEV-547687

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Continuous Integration

Technical details

Added/updated tests?

  • Yes
  • No, Does not apply to this PR.

Updated CHANGELOG?

  • Yes
  • No, Does not apply to this PR.

Added/Updated documentation?

  • Yes
  • No, Does not apply to this PR.

🔁 Imported from ROCm/rocprofiler-sdk#102
🧑‍💻 Originally authored by @rocm-devops

@ammarwa

ammarwa commented Aug 12, 2025

Copy link
Copy Markdown
Collaborator

Code Coverage Report

Code Coverage Report

Tests Only

code coverage tests.png

Samples Only

code coverage samples.png

Tests + Samples

code coverage all.png

@yhuiYH yhuiYH left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably want to get rid of get_temp_view_names function too. That'll prove that we don't need any views.

Recall, Jonathan suggested to replace any temp views and instead, use a "WITH" query in SQL.

I know, it's probably a more significant re-write, which may or may not be worth it at this point. We may want to focus on different task first.

Comment thread projects/rocprofiler-sdk/source/lib/python/rocpd/summary.py Outdated
Comment thread projects/rocprofiler-sdk/source/lib/python/rocpd/summary.py Outdated
@amd-atumakae amd-atumakae force-pushed the atumakae/summary_sql_queries branch from 3e5ba67 to a2693be Compare August 18, 2025 14:09
@amd-atumakae amd-atumakae requested a review from yhuiYH August 18, 2025 17:05
systems-assistant Bot pushed a commit that referenced this pull request Aug 18, 2025
Co-authored-by: Rahul Manocha <rmanocha@amd.com>
jayhawk-commits pushed a commit that referenced this pull request Aug 18, 2025
Co-authored-by: Rahul Manocha <rmanocha@amd.com>

[ROCm/hip commit: c9b9717]
@amd-atumakae amd-atumakae force-pushed the atumakae/summary_sql_queries branch from a2693be to d045a71 Compare August 20, 2025 11:09
Comment thread projects/rocprofiler-sdk/source/lib/python/rocpd/summary.py Outdated
Comment thread projects/rocprofiler-sdk/source/lib/python/rocpd/summary.py Outdated
@amd-atumakae amd-atumakae force-pushed the atumakae/summary_sql_queries branch 2 times, most recently from 1b9926f to b58e81e Compare August 21, 2025 15:02

@yhuiYH yhuiYH left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These are all just minor nit pick text suggestions.

Comment thread projects/rocprofiler-sdk/source/lib/python/rocpd/summary.py Outdated
Comment thread projects/rocprofiler-sdk/source/lib/python/rocpd/summary.py Outdated
Comment thread projects/rocprofiler-sdk/source/lib/python/rocpd/summary.py Outdated
Comment thread projects/rocprofiler-sdk/source/lib/python/rocpd/summary.py Outdated
Comment thread projects/rocprofiler-sdk/source/lib/python/rocpd/summary.py Outdated
Comment thread projects/rocprofiler-sdk/source/lib/python/rocpd/summary.py Outdated
@amd-atumakae amd-atumakae force-pushed the atumakae/summary_sql_queries branch from b58e81e to f3a7611 Compare August 25, 2025 12:51
@amd-atumakae amd-atumakae marked this pull request as ready for review August 25, 2025 12:56
@yhuiYH

yhuiYH commented Aug 26, 2025

Copy link
Copy Markdown
Contributor

It looks like summary.py test is failing on SLES (SUSE OS). I re-ran the test to see if it is still failing, but it's queued.
https://github.com/ROCm/rocm-systems/actions/runs/17209385347/job/48875551902?pr=311

It was failing on the |= operation you used. Maybe we need an alternative so it works across OSes (Ubuntu, RHEL and SLES)

@amd-atumakae amd-atumakae force-pushed the atumakae/summary_sql_queries branch from f3a7611 to 8820edd Compare August 26, 2025 12:18

@yhuiYH yhuiYH left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK, please resolve any PR comments once you've addressed them. It also makes the PR look cleaner and easier for others to read.

@amd-atumakae amd-atumakae force-pushed the atumakae/summary_sql_queries branch from 8820edd to b5a8dd8 Compare August 28, 2025 20:47
@yhuiYH

yhuiYH commented Sep 4, 2025

Copy link
Copy Markdown
Contributor

I think this one is ready to merge, but we can't merge due to PSDB failing (unrelated tests)

@yhuiYH yhuiYH merged commit 646e4d2 into develop Sep 15, 2025
30 of 38 checks passed
@yhuiYH yhuiYH deleted the atumakae/summary_sql_queries branch September 15, 2025 21:13
systems-assistant Bot pushed a commit to ROCm/rocprofiler-sdk that referenced this pull request Sep 15, 2025
 (#311)

* Use queries instead of views in summary.py

* Export queries when created

* Remove HIP and HSA from output

* Fix domain query

* Export summary queries in the main function

* Fix comments and variable names

* Change syntax for old python versions
[rocm-systems] ROCm/rocm-systems#311 (commit 646e4d2)
sauverma93 pushed a commit that referenced this pull request Sep 26, 2025
* Use queries instead of views in summary.py

* Export queries when created

* Remove HIP and HSA from output

* Fix domain query

* Export summary queries in the main function

* Fix comments and variable names

* Change syntax for old python versions

---------

Co-authored-by: Young Hui <young.hui@amd.com>
ryang-amd pushed a commit that referenced this pull request Nov 7, 2025
* Use queries instead of views in summary.py

* Export queries when created

* Remove HIP and HSA from output

* Fix domain query

* Export summary queries in the main function

* Fix comments and variable names

* Change syntax for old python versions

---------

Co-authored-by: Young Hui <young.hui@amd.com>
ammallya pushed a commit that referenced this pull request Nov 17, 2025
Changes:
- Changed HIP_UUID to reference rsmi_dev_unique_id_get()
- Added better logging for amdsmi_get_gpu_device_uuid() references

Change-Id: Ie233044de8c6e85b807faf22121b450233db861b

Signed-off-by: Charis Poag <Charis.Poag@amd.com>
ammallya pushed a commit that referenced this pull request Nov 18, 2025
Changes:
- Changed HIP_UUID to reference rsmi_dev_unique_id_get()
- Added better logging for amdsmi_get_gpu_device_uuid() references

Change-Id: Ie233044de8c6e85b807faf22121b450233db861b

Signed-off-by: Charis Poag <Charis.Poag@amd.com>

[ROCm/amdsmi commit: 817c077]
ammallya pushed a commit that referenced this pull request Nov 21, 2025
Changes:
- Changed HIP_UUID to reference rsmi_dev_unique_id_get()
- Added better logging for amdsmi_get_gpu_device_uuid() references

Change-Id: Ie233044de8c6e85b807faf22121b450233db861b

Signed-off-by: Charis Poag <Charis.Poag@amd.com>

[ROCm/amdsmi commit: 817c077]
ammallya pushed a commit that referenced this pull request Jan 21, 2026
A copy paste mistake in a previous commit caused source and dest to
be reversed.  Correct the source and dest params.

Fixes: 6de67d5

Signed-off-by: Allen Hubbe <allen.hubbe@amd.com>
ammallya pushed a commit that referenced this pull request Jan 21, 2026
A copy paste mistake in a previous commit caused source and dest to
be reversed.  Correct the source and dest params.

Fixes: e8a7371

Signed-off-by: Allen Hubbe <allen.hubbe@amd.com>

[ROCm/rocshmem commit: e2dcf99]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants