Skip to content

Always build and run Cython tests + other CI improvements #640

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

Merged
merged 32 commits into from
May 19, 2025

Conversation

leofang
Copy link
Member

@leofang leofang commented May 16, 2025

Description

Previously, the Cython tests are run on

  • Latest CUDA 11.x (11.8.0) and 12.x (12.9.0)
  • Linux only

With this PR, the Cython tests are run on

  • Latest CUDA 12.x (12.9.0)
  • Linux and Windows

We can no longer run against the previous major version because of the way of building the Cython tests is changed. It is now built against the build-time CTK, which is always the latest major.minor. But I think this is OK, because the same Cython tests are already run in each major version's branch (example from 11.x).

There is still one open task relevant to Cython tests (#274 (comment)), but the requirement there is likely incompatible with either the old or new approach here and always needs a separate job/workflow, so I consider it out of scope.

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@leofang leofang self-assigned this May 16, 2025
@leofang leofang added CI/CD CI/CD infrastructure cuda.bindings Everything related to the cuda.bindings module labels May 16, 2025
Copy link
Contributor

copy-pr-bot bot commented May 16, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@leofang
Copy link
Member Author

leofang commented May 16, 2025

/ok to test f0d7333

@leofang
Copy link
Member Author

leofang commented May 16, 2025

/ok to test 18be719

@leofang
Copy link
Member Author

leofang commented May 16, 2025

/ok to test c41a39a

@leofang
Copy link
Member Author

leofang commented May 17, 2025

/ok to test 227e315

@leofang
Copy link
Member Author

leofang commented May 17, 2025

/ok to test

Copy link
Contributor

copy-pr-bot bot commented May 17, 2025

/ok to test

@leofang, there was an error processing your request: E1

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/

@leofang
Copy link
Member Author

leofang commented May 17, 2025

/ok to test 9fc20e1

@leofang
Copy link
Member Author

leofang commented May 17, 2025

/ok to test 512e613

@leofang
Copy link
Member Author

leofang commented May 17, 2025

/ok to test de4db55

@leofang
Copy link
Member Author

leofang commented May 17, 2025

/ok to test be78f1b

@leofang
Copy link
Member Author

leofang commented May 17, 2025

/ok to test a0f964d

@leofang
Copy link
Member Author

leofang commented May 17, 2025

/ok to test 19e5709

@leofang
Copy link
Member Author

leofang commented May 17, 2025

/ok to test 78c8afd

@leofang
Copy link
Member Author

leofang commented May 18, 2025

/ok to test 8f7a054

@leofang
Copy link
Member Author

leofang commented May 18, 2025

/ok to test 9ad27ac

@leofang
Copy link
Member Author

leofang commented May 18, 2025

/ok to test 08c5579

@leofang
Copy link
Member Author

leofang commented May 19, 2025

/ok to test 452fced

@leofang
Copy link
Member Author

leofang commented May 19, 2025

/ok to test d6ada33

@leofang
Copy link
Member Author

leofang commented May 19, 2025

/ok to test f5ffde9

@leofang leofang added this to the cuda.core beta 4 milestone May 19, 2025
@leofang leofang added the P0 High priority - Must do! label May 19, 2025
@leofang
Copy link
Member Author

leofang commented May 19, 2025

/ok to test a676eb3

@leofang
Copy link
Member Author

leofang commented May 19, 2025

/ok to test f409d19

@leofang leofang changed the title Always build and run Cython tests Always build and run Cython tests + other CI improvements May 19, 2025
@leofang leofang marked this pull request as ready for review May 19, 2025 03:56
@leofang leofang requested review from cryos and kkraus14 May 19, 2025 03:56
run: |
pip install $(ls ${{ env.CUDA_CORE_ARTIFACTS_DIR }}/*.whl)[test]
pushd ${{ env.CUDA_CORE_CYTHON_TESTS_DIR }}
bash build_tests.sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be nice to have these built by a more standard mechanism and avoid needing to run a separate bash script. Any reason we wouldn't want these built when invoking pip install?

Doesn't need to be solved in this PR regardless.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I had the same thought, I have some local changes (started on build) I will rebase once this is merged.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I don't want to include the built tests in the wheel. If there is a way to achieve this with the build frontend/backend, it'd certainly simplify build-wheel.yml here, but I couldn't find any.

Copy link
Collaborator

@cryos cryos left a comment

Choose a reason for hiding this comment

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

LGTM

@leofang leofang merged commit 41fdb86 into NVIDIA:main May 19, 2025
53 checks passed
@leofang leofang deleted the cython_test branch May 19, 2025 15:23
Copy link

Doc Preview CI
Preview removed because the pull request was closed or merged.

@leofang leofang mentioned this pull request Jun 12, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD CI/CD infrastructure cuda.bindings Everything related to the cuda.bindings module P0 High priority - Must do!
Projects
None yet
3 participants