-
Notifications
You must be signed in to change notification settings - Fork 21
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
Windows CI failures #1211
Comments
Possibly just a bad CI image, failures were using I've retriggered one of the failed workflows to see if the issues has magically gone away again. https://github.com/FLAMEGPU/FLAMEGPU2/actions/runs/9463757119/job/27223363960 Not fixed by Will be simpler to debug in a fork or less complex repository (i.e. ptheywood/cuda-cmake-github-actionsafter some updates) |
From triggering a CI run on the above repo, it appears that CUDA is deciding the version of visual studio is incompatible when CMake is triggering the test build:
However it says 2022 inclusive should be fine. But it seems that this might be caused by Microsoft changing the numeric version to 19.40, but nvidia assumed that 1940 would be visual studio 202X. CUDA 12.5 should not get angry, but older ones still will. We need to tell CMake to use |
Might also require a CMake >= 3.29.4 for compiler detection to use the user provided flags / 3.29.4 will fix the issue anyway (but users will need to specify allowing unsupported compilers too) if I'm understanding this correctly: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9546 So it might just be that on windows, if using MSVC >= whichever version this is, and CUDA < We can probably emit an appropriate error message somewhere like we do for other compiler issues if all these conditions are met (and a compiler check fails). FLAMEGPU2/cmake/CheckCompilerFunctionality.cmake Lines 22 to 26 in 4e02604
EditYep
|
Final CI runs I'd set going on Friday confirm that CUDA <= 12.3 with So (ideally) the warning needs to be emitted when:
It should tell users that:
Could also be worth adding to installation instructions for MSVC users or somewhere. It's not an us issue, it's a generic CUDA < 12.4 && MSVC_VERISON >= 1940 (but still VS 2022) issue. It would be worth updating our CI to include CUDA 11.x, 12.0 & 12.5 builds at the same time to cover this. |
Possible problem: CMake's fix to pass It does not appear to work when using This is how we optionally enable CUDA (to allow docs only builds on CI for instance) and is required if we want to even stand a chance of providing a helpful error message about this. From looking at the generated
because it is not passing My previous standalone project CI runs which showed that would be a fix did not use I'll report this upstream when I have time. For FLAME GPU 2 short term, we have quite limited options:
These errors occur even if |
Although So we can fix this in our CI (and suggest this to windows users). Although the warnings we can provide will not be as useful as I'd hoped (Can't find out which CUDA versions CMake tried and failed to use, without reading random files off disk which feels brittle and more effort than it is worth) Per configuration it can also be set using So we can workaround this without needing any fixes upstream (although I might comment on the original issue that if using Todo
|
My previous fix which worked last week appears to have stopped working on CI after an update 👍 I.e. PR on the vis repo worked, CI after merge failed. MSVC has updated to 1941 from 1940, which stands out as a possible culprit. CMake version may also have changed, but that is not obviously logged (we use the default in most CI places, perhaps we want to fix to a known version for stability, at the cost of hiding new problems) Set a standalone workflow going to see if I can reproduce, otherwise will need to update a local install and see if I can reproduce and find a fix. |
Microsofts standard library distributed with MSVC 1941 now issues a static assertion when CUDA is < 12.4, introduced in This means that we need to ammend our user-facing error message if MSVC >= 1941 and cuda language support not found, then if they are attempting to use CUDA <= 12.3 then they need to also define However, this would need to be defined during CMake CheckLanguage testing (as well as for all targets produced by users). |
At some point between 2024-04-26 and 2024-06-11 our Windows CI has stopped working, for CUDA 11.8 and CUDA 12.3 builds (11.0 builds are fine).
In both cases, CMake could not find CUDA language support:
This is after CUDA was installed successfully via network installer at the appropriate location and path updated
Probably worth debugging in a separate mcuh smaller repo to avoid CI spam / long CI times when it does work.
i.e. a fork of flamegpu with most CI disabled, or a fork of https://github.com/ptheywood/cuda-cmake-github-actions (the repo I used to develop the cuda install scripts in the first place).
Things to check could be:
nvcc
is correctly on the path just before the cmake configuration phasenvcc
can manually compile a test program without CMake (which is what cmake does)The text was updated successfully, but these errors were encountered: