-
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
CI: Add FLAMEGPU_SEATBELTS=OFF CI via matrix include: #1138
Conversation
This should fail currently, until #1137 is resolved. |
It seems that my existing, working use of complex objects within a matrix is not strictly correct/supported even though it works. In the web editor, the cudacxx objects are reported as an error.
I might have to figure out the "correct" way to do this in general (I.e. where using a specific cuda version means a specific value of cuda_arch/hostcxx should be used). It might be that we have to do a fully |
Would a simpler compromise be to configure ALL standalone example builds to be That would force a main library build, but wouldn't catch all device API template code (only going to fully get that with the test suite). It's possibly a suitable compromise, if release flow ensures tests are built |
That would be an option to quickly get a From some github issues / feature requests it seems that lists can be used as matrix elements, so potentially could use something like:
instead of
Though its a bit less readable / easy to make mistakes. Thankfully I did map matrix values to env vars where they are used, so the changes should only need to be prior to jobs starting. I'll mock up the matrix stuff in a separate repo when I have some time. |
This is a short-term / quick way to get a beltsoff build into CI, suggested by @Robadob as short term part of #1138. This will be replaced by a 'correct' version in the future (as our current matrix is technically unsupported, although it mostly works, and is not expliucitly documented as such other than in the web workflow editor
This is a short-term / quick way to get a beltsoff build into CI, suggested by @Robadob as short term part of #1138. This will be replaced by a 'correct' version in the future (as our current matrix is technically unsupported, although it mostly works, and is not expliucitly documented as such other than in the web workflow editor
…orkflows Using a number of exclude statemements, per OS the CI matrix should be: + 3 release console builds + 2 release vis builds + 1 beltsoff build The matrix YAML is not techincally supported by Github actions according to the web editor We should probably find a compliant way to do this, but it works so a problem for the future. The individual example uses the same (relevant) options as the main configuration.
e484459
to
6525577
Compare
This is now ready for review, assuming that CI passes. There are now 6 builds for regular Ubuntu and Windows CIs:
We could probably drop the CUDA 11.8 Release Vis build, as the vis shouldn't be significantly tied to the CUDA version, but it might be worth keeping. Adding a single GLM build might be worthwhile, but with the current matrix syntax that will be a lot of additional exclude statements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look sensible. Can merge assuming CI passes.
Useful sure, but not a priority whilst GLM remains experimental and not fully supported by Python. |
CI: Add FLAMEGPU_SEATBELTS=OFF CI via matrix include:
Using include is very verbose, but wanted to not multiply the exisiting matrix by 2, or trial and error expand the exclude region.
Switching to dynamic build matrices via a 2 step job would be nice but likely significant effort
Closes #1136
Todo:
Fix CI YAML to be valid according to the GitHub Workflow web editorDeferred to a separate issue CI Matrix Syntax #1145