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

Fix hybrid type for ROCm #192

Open
wants to merge 1 commit into
base: rocm-main
Choose a base branch
from
Open

Fix hybrid type for ROCm #192

wants to merge 1 commit into from

Conversation

Ruturaj4
Copy link

@Ruturaj4 Ruturaj4 commented Jan 6, 2025

No description provided.

@Ruturaj4 Ruturaj4 added the open-upstream Tag when you want a copy of this PR to be opened on upstream label Jan 6, 2025
Copy link

github-actions bot commented Jan 6, 2025

Feature branch from main is ready. Create a new PR destined for upstream?

@charleshofer charleshofer self-requested a review January 6, 2025 15:45
@charleshofer
Copy link
Collaborator

Looks like CI is failing due to this error: jax-ml#25664. Some deprecated scipy that upstream put in. There's a fix here. I suggest we just skip those tests until upstream gets the fix in: jax-ml#25675

@charleshofer
Copy link
Collaborator

Oh, I already fixed this upstream: https://github.com/jax-ml/jax/blame/main/jaxlib/tools/build_gpu_kernels_wheel.py#L148. The real question is how did we lose track of that change?

@charleshofer
Copy link
Collaborator

I fixed it downstream, too: #173

@charleshofer
Copy link
Collaborator

I don't understand how git is even generating that diff. In our rocm-main the file already points to the rocm directory, right? https://github.com/ROCm/jax/blame/rocm-main/jaxlib/tools/build_gpu_kernels_wheel.py#L148

@charleshofer
Copy link
Collaborator

It looks like the ci_hybrid_typo branch that you're using to make the PR split from upstream main like two months ago. That split happened before I pushed the fix to upstream, so it makes sense that when we try and open a PR that goes from ci_hybrid_typo-upstream to upstream main that there is no difference. What doesn't make sense to me is why when you open a PR from ci_hybrid_typo into rocm-main that GitHub shows a diff between the two branches. Anyways, unless I'm missing something, I think we can just close this PR.

@Ruturaj4 I'm not sure what the context is for you finding this bug, but if you use the latest rocm-main or the latest upstream main it's already fixed. Unless you're doing release work, it's generally not great to be using rocm-main or upstream main code from two months ago because you can end up re-debugging things like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-upstream Tag when you want a copy of this PR to be opened on upstream
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants