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

target branch-24.04 for GitHub Actions workflows #191

Merged
merged 2 commits into from
Feb 20, 2024

Conversation

jameslamb
Copy link
Member

@jameslamb jameslamb commented Feb 15, 2024

Description

Follow-up to #161

For all GitHub Actions configs, replaces uses of the test-cuda-12.2 branch on shared-workflows
with branch-24.04, now that rapidsai/shared-workflows#166 has been merged.

Notes for Reviewers

This is part of ongoing work to build and test packages against CUDA 12.2 across all of RAPIDS.

For more details see:

(created with rapids-reviser)

@jameslamb jameslamb added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Feb 15, 2024
@jameslamb jameslamb marked this pull request as ready for review February 15, 2024 20:10
@jameslamb jameslamb requested a review from a team as a code owner February 15, 2024 20:10
@jakirkham
Copy link
Member

Thanks James! 🙏

There was a network error in one job that caused the rest to cancel at that point. Restarted CI

@jakirkham
Copy link
Member

jakirkham commented Feb 15, 2024

Seeing flaky test failures. Restarting

Edit: Restarting again x2

Copy link
Member

@pentschev pentschev left a comment

Choose a reason for hiding this comment

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

LGTM, I'm just wondering if this is ready to merge or if there are other requirements, I noticed some tests have started failing due to the lack of this change:

ValueError: No matching matrix found in 'cuda_version' for: {'cuda': '12.2', 'arch': 'x86_64', 'py': '3.10'}

@jameslamb
Copy link
Member Author

This is ready to merge. It should be a no-op from the perspective of this project... the test-cuda-12.2 and branch-24.04 branches in https://github.com/rapidsai/shared-workflows are identical right now.


But @pentschev I see the error you're referring to, on #192 for example (build link)

Traceback (most recent call last):
  File "/opt/conda/bin/rapids-dependency-file-generator", line 8, in <module>
    sys.exit(main())
  File "/opt/conda/lib/python3.10/site-packages/rapids_dependency_file_generator/cli.py", line 104, in main
    make_dependency_files(parsed_config, args.config, to_stdout)
  File "/opt/conda/lib/python3.10/site-packages/rapids_dependency_file_generator/rapids_dependency_file_generator.py", line 4[29](https://github.com/rapidsai/ucxx/actions/runs/7931704367/job/21657022853?pr=192#step:7:30), in make_dependency_files
    raise ValueError(
ValueError: No matching matrix found in 'cuda_version' for: {'cuda': '12.2', 'arch': 'x86_64', 'py': '3.10'}
Error: Process completed with exit code 1.

Can you please update that branch to the latest branch-0.37? I see that dependencies.yaml on that PR's branch is missing the recent changes from #161 (https://github.com/pentschev/ucxx/blob/request-endpoint-close/dependencies.yaml). That should resolve that issue.

@pentschev
Copy link
Member

@jameslamb you're right, merging branch-0.37 did fix that. Thanks for pointing that out!

@jameslamb
Copy link
Member Author

No problem!

I've noticed that other RAPIDS repos have "always suggest updating pull request branches" checked in their settings:

image

Which causes a button to show up down in the checks & merge section showing that the branch isn't up to date with the target, like this:

Screenshot 2024-02-16 at 2 42 17 PM

I don't believe I saw that earlier when I looked at #192, so I suspect this repo doesn't have that enabled. I recommend enabling that setting, I've found it super useful!

@pentschev
Copy link
Member

I don't believe I saw that earlier when I looked at #192, so I suspect this repo doesn't have that enabled. I recommend enabling that setting, I've found it super useful!

I think it only does that after a certain number of PRs, at least that's what my past experience with that feature seemed like with various repos, i.e., if it's only behind for one or two commits that warning may not show up.

@raydouglass
Copy link
Member

I recommend enabling that setting, I've found it super useful!

I've enabled it

@pentschev
Copy link
Member

I'm seeing the following errors in repos where UCXX is required (namely Dask-CUDA and RAFT PR where UCXX support is being added):

  The following packages are incompatible
  ├─ cuda-version 12.0**  is requested and can be installed;
  └─ distributed-ucxx 0.37**  is not installable because it requires
     └─ ucxx >=0.37.0a,<0.38.0a0  but there are no viable options
        ├─ ucxx 0.37.00a would require
        │  └─ cuda-version >=11,<12.0a0 , which conflicts with any installable versions previously reported;
        └─ ucxx 0.37.00a would require
           └─ cuda-version >=12.2,<13 , which conflicts with any installable versions previously reported.

I'm assuming this PR will resolve that, is that right @jameslamb ? Is there anything else that needs to be done here or are we good to merge it?

@jameslamb
Copy link
Member Author

This is good to merge any time, but @pentschev it won't affect that issue you've linked to.

This PR just updates which branch of https://github.com/rapidsai/shared-workflows GitHub Actions configuration is pulled from, and the current branch (test-cuda-12.2) and new one (branch-24.04) are identical.

@pentschev
Copy link
Member

Thanks @jameslamb , following up on that discussion offline.

@pentschev
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit e011f42 into rapidsai:branch-0.37 Feb 20, 2024
49 checks passed
@jameslamb
Copy link
Member Author

thanks!

@jameslamb jameslamb deleted the gha-cuda-12.2 branch February 20, 2024 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants