Fix ROCpd dependencies in Python wheels by srihari-uttanur · Pull Request #9535 · ROCm/TheRock

srihari-uttanur · 2026-10-27T17:07:45Z

Design discussion: #9788

The dependency-source policy and profiler-selector behavior remain pending maintainer agreement. The current pushed revision uses PyPI; the proposed revision targets ROCm-index-only installs.

Motivation

JIRA ID: AIPROFSDK-1189

ROCpd help works in a clean wheel installation, but CSV conversion and summaries require pandas. Keep the core package dependency-free while making the opt-in rocm[profiler] installation usable for analysis. This complements #9509's command/help fixes.

Technical Details

Add Linux-only core extras: rocpd (pandas), rocpd-dashboard (pandas, jinja2), rocpd-pdf (pandas, reportlab), and rocpd-otf2 (otf2).
Have rocm[profiler] request rocm-sdk-core[rocpd] at the matching version. Plain core, rocm, libraries, and devel installs do not acquire pandas.
Follow fix(rocprofiler-compute): Gate rocprof-compute analyze deps behind an opt-in compute-analyze extra #7002's opt-in PyPI approach. Remove the proposed mirror/routing changes and the PyPI fallback from ordinary core installs.
Keep exporters separate instead of importing the SDK's combined runtime requirements, which would make analysis depend on OTF2 availability and include the obsolete dataclasses backport.
Add built-wheel metadata, selector, and real CSV/summary regressions. Remove the ROCpd wheel test's skip, explicitly require wheel for tests, and update installation instructions.

No SDK sources, submodule pointers, native build targets, or command entry points are changed. Dependency additions do not activate on Windows/macOS. Existing #9509 behavior is retained after rebasing onto main.

Analysis adds approximately 123.25 MiB of installed third-party files in the tested Python 3.10 environment; base installs add none. This is not download size. Dependencies remain separately distributed under their upstream licenses. Packaging ownership remains with its CODEOWNERS and functional requirements with SDK maintainers. Full-build and install-time performance were not benchmarked.

Test Plan

python -B -m pytest -q \
  build_tools/tests/py_packaging_test.py \
  build_tools/tests/setup_venv_test.py \
  build_tools/packaging/python/tests/mirror_python_dependencies_test.py \
  build_tools/packaging/python/tests/aggregate_index_test.py

Run configured pre-commit hooks, scoped security scans, and fresh-venv conversion/exporter checks. The installed-wheel regression creates two CPU-side API records, invokes CSV conversion and summary generation, and checks exported rows and summarized calls.

Test Result

239 tests and 37 subtests pass on Python 3.13.18.
Applicable pre-commit hooks, actionlint, scoped Bandit, offline zizmor, and whitespace checks pass.
Native CSV/summary checks pass on Python 3.10 and manylinux Python 3.13. Removing pandas fails the regression; restoring the same version restores the pass on both interpreters.
Base-install isolation, help, and Perfetto pass on Python 3.10. Independent dashboard/PDF checks pass on both interpreters; OTF2 passes on Python 3.10. Final clean environments pass pip check.
Fresh GitHub CI and maintainer approval.

Validation uses real templates with a native ROCpd payload and synthetic package version, not a full ROCm rebuild. Native Windows/macOS and OTF2 on newer Python were not tested. A standalone-Python 3.13 SQLite-symbol incompatibility reproduces against the untouched runtime; standard manylinux Python 3.13 passes without a workaround. No GPU workloads were launched. Rebuilt/published wheels are required to deliver the metadata changes.

Submission Checklist

Read the contributing guidelines and add regression coverage.
Publish/link the requested public design issue and agree on profiler-selector forwarding and PyPI versus mirroring.
Pass required GitHub checks on the final revision and obtain review approval.

AI assistance: GitHub Copilot assisted with implementation, validation, and PR preparation.

therock-pr-bot · 2026-10-27T17:08:11Z

✅ All Checks Passed — Ready for Review

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🔎 pre-commit	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—
🤖 therock-pr-bot	✅ Pass	—

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

therock-pr-bot · 2026-10-27T17:08:12Z

🎉 All checks passed! This PR is ready for review.

ScottTodd

This needs a well-motivated github issue where we can agree on the technical direction, before we can proceed further with code review.

I want to see specifically:

What ROCpd is, how it is used, what requirements it has
Discussion on whether these requirements should be mirrored to https://repo.amd.com/rocm/ (this PR includes code that installs from pypi)

This PR as is has around 3x the text needed to express the core idea in the description and first we need to decide if this is a problem worth solving and what approach to take to solve it.

ScottTodd · 2026-10-29T18:34:21Z

    },
    zip_safe=False,
    include_package_data=True,
+    install_requires=["pandas; sys_platform == 'linux'"],


Let's keep 0 install requirements for the rocm-sdk-core package. pandas should only be for an extra, especially since it also pulls in numpy, python-dateutil, six, and tzdata. If certain tools require additional packages they should fail gracefully and instruct the user to install those packages.

Currently, rocpd does fail gracefully and instruct the user to install the packages.

@srihari-uttanur , perhaps we should revisit this and take a look at what rocprof-compute has done, with their compute-analyze extra: https://github.com/ROCm/TheRock/pull/7002/changes

SDK is in core, not profilers so there's that nuance, but we should be able to do something similar?

This is also why I made a change to SDK's requirements.txt file, so that we can potentially mirror rocprof-compute's solution: ROCm/rocm-systems#12304, and install those packages from PyPi

I’ve followed the opt-in extras/PyPI pattern, with the extras on rocm-sdk-core because it owns ROCpd. The dependency groups are currently declared separately rather than read from the combined SDK requirements: importing that list wholesale would make CSV/summary installation depend on OTF2 availability and include the obsolete dataclasses backport. Dashboard, PDF, and OTF2 remain independently selectable.

@ScottTodd I removed the mandatory pandas requirement. The base rocm-sdk-core package remains dependency-free; pandas is now behind the Linux-only rocpd extra. rocm[profiler] requests that extra at the matching version, while plain core, libraries, and devel installs do not. The existing missing-dependency guidance is unchanged, and negative tests verify failure without pandas and recovery after installation.

ScottTodd · 2026-10-29T18:37:58Z

+    def test_core_setup_builds_wheel_with_rocpd_dependencies(self):
+        if importlib.util.find_spec("wheel") is None:
+            self.skipTest("wheel package is not available")


wheel already comes in via https://github.com/ROCm/TheRock/blob/main/requirements-test.txt (implicitly, we could add it explicitly). We should not skip tests.

Removed the skip and explicitly added wheel to the test requirements. The regression now always builds a real wheel and checks its dependency metadata. I also added an installed-wheel CSV/summary test that fails when pandas is missing. All 239 affected tests and 37 subtests pass locally.

Declare Linux pandas and independent exporter extras in the core wheel. Mirror required dependencies and add aggregate index routes, retaining Python 3.10-compatible releases. Cover wheel metadata, platform markers, wheel tags, and mirror routing.

Allow Linux artifact installs to resolve third-party dependencies from PyPI while preserving configured index and Windows behavior. Cover the index guard and core install steps with workflow regressions.

Keep core metadata dependency-free, forward the profiler selector to its analysis extra, and separate optional exporters. Limit PyPI to profiler installs and cover wheel metadata plus native CSV/summary conversions.

ScottTodd · 2026-11-05T23:47:31Z

+class WheelWorkflowTest(unittest.TestCase):
+    def setUp(self):
+        workflow_path = (
+            Path(__file__).resolve().parents[2]
+            / ".github/workflows/test_rocm_wheels.yml"
+        )
+        self.job = yaml.safe_load(workflow_path.read_text())["jobs"]["test_wheels"]
+
+    def test_core_install_steps_do_not_add_a_dependency_index(self):
+        self.assertNotIn("ARTIFACT_DEPENDENCY_INDEX_URL", self.job["env"])
+        steps = {step.get("id"): step for step in self.job["steps"]}
+        for step_id in ("install_libraries", "install_devel"):
+            with self.subTest(step=step_id):
+                command = shlex.split(steps[step_id]["run"])
+                self.assertFalse(
+                    any(
+                        argument.startswith("--extra-index-url") for argument in command
+                    )
+                )
+                self.assertIn("--index-url=${PACKAGE_INDEX_URL}", command)
+                self.assertIn("--find-links=${PACKAGE_FIND_LINKS_URL}", command)
+
+    def test_profiler_install_resolves_optional_dependencies_from_pypi(self):
+        step = next(
+            step for step in self.job["steps"] if step.get("id") == "install_profiler"
+        )
+        self.assertEqual(step["if"], "${{ !cancelled() && runner.os == 'Linux' }}")
+        command = shlex.split(step["run"])
+        self.assertIn("rocm[profiler]==${ROCM_VERSION}", command)
+        self.assertIn("--index-url=${PACKAGE_INDEX_URL}", command)
+        self.assertIn("--find-links=${PACKAGE_FIND_LINKS_URL}", command)
+        self.assertIn("--extra-index-url=https://pypi.org/simple", command)
+
+


These tests are not useful and tests for github actions workflow do not belong in setup_venv_test.py. The tests just check the text contents of a .yml file. The text doesn't matter, what matters is that the workflow runs with expected behavior, and CI already does that.

See:

https://github.com/ROCm/TheRock/blob/main/TESTING.md#github-actions-workflows---validation-methods

https://github.com/ROCm/TheRock/blob/main/TESTING.md#python-scripts-and-tools---validation-methods

We do have some tests for github actions workflows in https://github.com/ROCm/TheRock/tree/main/build_tools/github_actions/tests starting with workflow_* but these are general purpose or enforce meaningful invariants that running the tests would not catch.

Done, I have removed them.

ScottTodd · 2026-11-05T23:51:57Z

+As with `rocm-profiler[compute-analyze]`, third-party dependencies come from
+PyPI. Add `--extra-index-url=https://pypi.org/simple` when the primary index is
+the ROCm index. These extras do not depend on new AMD mirror routes. Offline
+installations must supply the selected dependencies in their wheelhouse.


In general we should support installing all packages from our index pages without relying on the external pypi index. This would need a good reason to do something different.

This needs a well-motivated github issue where we can agree on the technical direction, before we can proceed further with code review.

https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md#communication-channels

https://github.com/ROCm/TheRock/blob/main/CONTRIBUTING.md#using-github-issues-for-feature-development

I want to see specifically:

What ROCpd is, how it is used, what requirements it has

Discussion on whether these requirements should be mirrored to https://repo.amd.com/rocm/ (this PR includes code that installs from pypi)

I don't see an issue linked where the design discussions should happen. Can you add that prominently in the PR description?

created this github issue 9788
Updated the description as well.

Mirror direct and transitive exporter requirements while retaining Python 3.10-compatible wheels. Remove the profiler PyPI fallback and workflow-text assertions. Cover wheel filtering, snapshot retention, and generated routes.

srihari-uttanur requested review from ScottTodd and marbre as code owners October 27, 2026 17:07

github-project-automation Bot added this to TheRock Triage Oct 27, 2026

github-project-automation Bot moved this to TODO in TheRock Triage Oct 27, 2026

therock-pr-bot Bot added the Not ready to Review PR has unresolved policy failures — reviews blocked label Oct 27, 2026

therock-pr-bot Bot removed the Not ready to Review PR has unresolved policy failures — reviews blocked label Oct 27, 2026

ScottTodd requested changes Oct 29, 2026

View reviewed changes

ScottTodd mentioned this pull request Oct 29, 2026

fix(profilers): fix rocprof-attach and add tests for rocpd and rocprofv3 executables #9509

Merged

1 task done

Srihari, Uttanur added 3 commits November 3, 2026 07:50

Fix dependency resolution in artifact wheel tests

9c83de7

Allow Linux artifact installs to resolve third-party dependencies from PyPI while preserving configured index and Windows behavior. Cover the index guard and core install steps with workflow regressions.

Make ROCpd wheel analysis dependencies opt-in

2a4093c

Keep core metadata dependency-free, forward the profiler selector to its analysis extra, and separate optional exporters. Limit PyPI to profiler installs and cover wheel metadata plus native CSV/summary conversions.

srihari-uttanur force-pushed the users/srihariu/fix-rocpd-wheel-dependencies branch from a10e85d to 2a4093c Compare November 3, 2026 14:08

srihari-uttanur requested review from ScottTodd and yhuiYH November 5, 2026 11:21

ScottTodd reviewed Nov 5, 2026

View reviewed changes

srihari-uttanur mentioned this pull request Nov 6, 2026

Define ROCm-index-only installation for ROCpd Python extras #9788

Open

Resolve ROCpd extras from the ROCm index

c4ab477

Mirror direct and transitive exporter requirements while retaining Python 3.10-compatible wheels. Remove the profiler PyPI fallback and workflow-text assertions. Cover wheel filtering, snapshot retention, and generated routes.

Conversation

srihari-uttanur commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

Test Plan

Test Result

Submission Checklist

Uh oh!

therock-pr-bot Bot commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ All Checks Passed — Ready for Review

Uh oh!

therock-pr-bot Bot commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ScottTodd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srihari-uttanur Nov 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srihari-uttanur Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

srihari-uttanur commented Oct 27, 2026 •

edited

Loading

therock-pr-bot Bot commented Oct 27, 2026 •

edited

Loading

therock-pr-bot Bot commented Oct 27, 2026 •

edited

Loading

srihari-uttanur Nov 3, 2026 •

edited

Loading

srihari-uttanur Nov 6, 2026 •

edited

Loading