Repository navigation
ci: Initial pass at GPU testing for cutile-rs - #1607
assignUser wants to merge 7 commits into
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test 041e338 |
|
/ok to test 7a50fa8 |
|
/ok to test fcac53a |
|
/ok to test 495ff7e |
495ff7e to
fd7cf70
Compare
|
/ok to test fd7cf70 |
1 similar comment
|
/ok to test fd7cf70 |
|
/ok to test 8454bb6 |
|
/ok to test 8454bb6 |
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
8454bb6 to
063b3a8
Compare
|
/ok to test 063b3a8 |
1 similar comment
|
/ok to test 063b3a8 |
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
Signed-off-by: Jacob Wujciak-Jens <[email protected]>
|
/ok to test ba76459 |
cryos
left a comment
cryos
left a comment
There was a problem hiding this comment.
There are a few minor things I pointed out inline that would be nice to improve. I think this looks good. A follow up would be to generalize a little more, add a matrix for more GPUs but I think getting this merged first would be best.
| path: ${{ runner.temp }}/cutile-gpu-tests.tar | ||
| if-no-files-found: error | ||
| compression-level: 0 # The nested nextest archives are already compressed. | ||
| retention-days: 7 |
There was a problem hiding this comment.
We might want to reduce this to 1 day, we can easily rebuild.
| - name: Upload runtime test packages | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: cutile-gpu-tests |
There was a problem hiding this comment.
I don't think we need it now, but consider adding OS and CPU architecture.
There was a problem hiding this comment.
Yeah, if we make a matrix that will be necessary. Otherwise there will be clobbering 🔨
| runs-on: linux-amd64-gpu-rtxpro6000-latest-1 | ||
| timeout-minutes: 30 | ||
| container: | ||
| image: nvidia/cuda:13.3.0-devel-ubuntu24.04 |
There was a problem hiding this comment.
Maybe this has changed, but it wouldn't hurt to keep for now:
env:
NVIDIA_VISIBLE_DEVICES: ${{ env.NVIDIA_VISIBLE_DEVICES }}
for the container env. I checked and it has in fact changed. Where we see this we should actively remove it - it won't hurt but extra lines being carried around that are no longer needed.
| nvidia-smi | ||
| "$CUTILE_TILEIRAS_PATH" --version | ||
|
|
||
| # Mirror scripts/run_gpu_tests.sh's runtime selection. Archive mode uses |
There was a problem hiding this comment.
I wonder if there is a neater way to do this using the script too, perhaps not but it is a concern that they get out of sync.
There was a problem hiding this comment.
I had this thought as well but just wanted to see if everything runs through for now
| - name: Submission lifetime tests (default, reactor, spin) | ||
| run: | | ||
| for budget in default 0 200000; do | ||
| echo "CUDA_ASYNC_SPIN_BUDGET_US=$budget" |
There was a problem hiding this comment.
Should prefer {} for bash variables too, I usually just ask Codex to add them to bash variable expansion.
There was a problem hiding this comment.
I don't think that's really necessary if you have nothing directly following the variable like here but if thats the preference 🤷
Summary
Changes
Testing
just -f cuda-oxide/Justfile checkpasses (the local mirror of CI: fmt, clippy, tests, guards, docs)cargo oxide run <example>passes, orcuda-oxide/scripts/smoketest.sh -o '^<example>$'Checklist
git commit -s)