Skip to content

[ci] - Adding gate check for CK#1181

Closed
geomin12 wants to merge 6 commits into
mainfrom
users/geomin12/monorepo-ck
Closed

[ci] - Adding gate check for CK#1181
geomin12 wants to merge 6 commits into
mainfrom
users/geomin12/monorepo-ck

Conversation

@geomin12
Copy link
Copy Markdown
Contributor

@geomin12 geomin12 commented Aug 2, 2025

Request from Joseph and Stella to add gate check for CK

Works here: https://github.com/ROCm/rocm-libraries/actions/runs/16684936432/job/47232156784

Copy link
Copy Markdown
Member

@marbre marbre left a comment

Choose a reason for hiding this comment

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

CK at HEAD isn't guaranteed to work with MIOpen at HEAD from rocm-libraries. With TheRock switching from individual subcomponents to the superrepo, this likely regularly breaks the TheRock build. Instead TheRock needs to consume a specific CK commit to which the submodule is pinned to, as via

def pin_ck():
requirements_file_path = THEROCK_DIR / "ml-libs" / "MIOpen" / "requirements.txt"
with open(requirements_file_path) as requirements_file:
requirements = requirements_file.read().splitlines()
# The requirements file pins several dependencies. And entry for CK looks like:
# 'ROCm/composable_kernel@778ac24376813d18e63c9f77a2dd51cf87eb4a80 -DCMAKE_BUILD_TYPE=Release'
# After filtering, the string is split to isolate the CK commit.
ck_requirement = list(
filter(lambda x: "rocm/composable_kernel" in x.lower(), requirements)
)[0]
ck_commit = ck_requirement.split("@")[-1].split()[0]
exec(
["git", "checkout", ck_commit],
cwd=THEROCK_DIR / "ml-libs" / "composable_kernel",
)

What CK rather needs is the ability to run test itself at HEAD with a TheRock build that consumes rocm-libraries but not in the way this patch would work.

@ScottTodd ScottTodd removed their request for review August 4, 2025 18:09
@ScottTodd
Copy link
Copy Markdown
Member

I'll defer to Marius here. I'm not as familiar with CK.

@marbre
Copy link
Copy Markdown
Member

marbre commented Aug 4, 2025

I'll defer to Marius here. I'm not as familiar with CK.

We talked offline and I will open a PR to allow @geomin12 addressing this in a workflow soon.

@geomin12
Copy link
Copy Markdown
Contributor Author

geomin12 commented Aug 4, 2025

Closing as this is not the correct requirements needed, should be applied to CK repo

@geomin12 geomin12 closed this Aug 4, 2025
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Aug 4, 2025
@geomin12 geomin12 deleted the users/geomin12/monorepo-ck branch August 4, 2025 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants