FLAMEGPU / FLAMEGPU2

FLAME GPU 2 is a GPU accelerated agent based modelling framework for CUDA C++ and Python
https://flamegpu.com
MIT License
99 stars 19 forks source link

CI: Add FLAMEGPU_SEATBELTS=OFF CI via matrix include: #1138

Closed ptheywood closed 8 months ago

ptheywood commented 8 months ago

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:

ptheywood commented 8 months ago

This should fail currently, until #1137 is resolved.

ptheywood commented 8 months ago

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.

Matrix options must only contain primitive values

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 include based matrix for that to be possible, which will quickly become incredibly verbose for wheel generation...

Robadob commented 8 months ago

Would a simpler compromise be to configure ALL standalone example builds to be SEATBELTS=OFF?

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 SEATBELTS=OFF too

ptheywood commented 8 months ago

That would be an option to quickly get a FLAMEGPU_SEATBELTS=OFF build covered, but the matrix yaml I've used everywhere for the cuda compiler would still be invalid.

From some github issues / feature requests it seems that lists can be used as matrix elements, so potentially could use something like:

` - compilerstuff: ["12.3", "50", gcc-12, ubuntu-22.04]

instead of

- cuda: "12.3"
  cuda_arch: "50"
  hostcxx: gcc-12
  os: ubuntu-22.04

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.

ptheywood commented 8 months ago

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.

Robadob commented 8 months ago

Adding a single GLM build might be worthwhile, but with the current matrix syntax that will be a lot of additional exclude statements.

Useful sure, but not a priority whilst GLM remains experimental and not fully supported by Python.