CMake option/cache variable naming

[ptheywood]

Refactor CMake: Option names, use CMAKE_CUDA_ARCHITECTURES, general improvments

• Adds top-level CMakeLists.txt detection to the root CMakeLists.txt
• Prefixes CMake options / CACHE variables with FLAMEGPU_ to cooperate with others, including:
  • BUILD_EXAMPLE_ -> FLAMEGPU_BUILD_EXAMPLE_ as dependent options
  • FLAMEPGU_BUILD_ALL_EXAMPLES -> FLAMEGPU_BUILD_ALL_EXAMPLES
    • Defaults to ON only if the main FLAMEGPU CMakeLists is the top level, removing the need for NO_EXAMPLES
  • BUILD_SWIG_PYTHON -> FLAMEPGU_BUILD_PYTHON
  • USE_NVTX -> FLAMEGPU_ENABLE_NVTX
  • USE_GLM -> FLAMEGPU_ENABLE_GLM
  • CURAND_ENGINE -> FLAMEGPU_CURAND_ENGINE, including a dropdown in the GUI
  • Removes USE_NVCC_THREADS
  • NVCC_THREADS -> FLAMEGPU_NVCC_THREADS which defaults to 2, and handles being set to OFF correctly.
  • VERBOSE_PTXAS -> FLAMEGPU_VERBOSE_PTXAS
  • BUILD_TESTS -> FLAMEGPU_BUILD_TESTS
  • BUILD_TESTS_DEV -> FLAMEGPU_BUILD_TESTS_DEV
  • EXPORT_RTC_SOURCES -> FLAMEGPU_RTC_EXPORT_SOURCES
  • RTC_DISK_CACHE -> FLAMEGPU_RTC_DISK_CACHE
  • WARNINGS_AS_ERRORS -> FLAMEGPU_WARNINGS_AS_ERRORS
  • SEATBELTS -> FLAMEGPU_SEATBELTS
  • BUILD_API_DOCUMENTATION -> FLAMEGPU_BUILD_API_DOCUMENTATION
  • VISUALIASTION -> FLAMEGPU_VISUSALISATION
    • And associated sub-optons FLAMEGPU_VISUALISATION_ROOT etc.
  • USE_GTEST_DISCOVER -> FLAMEGPU_ENABLE_GTEST_DISCOVER
• Switch to CMAKE_CUDA_ARCHITECTURES from CUDA_ARCH with custom default setting
• Mark many CMake Cache varaibles as advanced to exclude them from the default CMake GUI
• Improves cpplint CMake logic
• Move definition location of some CMake options, so they are pulled into individual examples
• Prefix Cmake functions and macros with flamegpu_ and rename to all lower case, including:
  • add_flamegpu_executable -> flamegpu_add_executable
• Use target-specific c++17 specification
• Correct cmake_minimum_required statements and add a <polciy_max> of 3.25
• Modernise GLM CMake
• De-duplicate compiler definition for the pyflamegpu target, using properties from the flamegpu target
• Reduce use of cmake variables from the visualiser CMake, instead request a list of (dll) files to copy is needed
• set target compile options on targets where possible
• Prefixes preprocessor macros with FLAMEGPU

Closes #837
Closes #976
Closes #448
Closes #732

[ptheywood]

The CMake GUI for a configuraiton with python, nvtx, tests and CUDA_ARCH=86 under linux currently looks like:

Which can be grouped

And ADVANCED exposes a bunch of options which most users shouldn't mind.

For now, I've left CUDA_ARCH alone, as it might be dissapearing (#976).

Similar changes need making to the vis repo, and the docs repo will need a tweak as well.

I've potentially still missed some CACHE variables / conditional options that are not in the top level CMake or other obvious locations, so I need to hunt those down.

---

@Robadob - any thoughts on FLAMEGPU_ENABLE_GLM instead of USE_GLM (and any other general cmake option name's that you'd like changing while i'm at it)?
Seems to be a mix of ENABLE vs USE in others, and apparently I was leaning towards ENABLE yesterday, maybe because it enables functionality rather than using it internally?

Renaming BUILD_SWIG_PYTHON to FLAMEGPU_BUILD_PYTHONis also maybe contentious, but we have no immediate plans for an alterante python implementation, which would 100% be a new major version anyway so I think its fine / slightly less typing for me (Though I've setup CMakeUserPresets.json which is quite nice to use)

---

I Might renable the NVCC_THREADS related options too, so they are neighbours in the list, not 100%.

[Robadob]

@Robadob - any thoughts on FLAMEGPU_ENABLE_GLM instead of USE_GLM (and any other general cmake option name's that you'd like changing while i'm at it)?
Seems to be a mix of ENABLE vs USE in others, and apparently I was leaning towards ENABLE yesterday, maybe because it enables functionality rather than using it internally?

No preference.

P.S. Not seen that grouping before.

[ptheywood]

P.S. Not seen that grouping before.

It's a checkbox next to the Advanced checkbox, just seems to group cache vars by splitting upto the first _.

The cmake-gui(1) tool can display options in groups defined by their prefix, so it makes sense for third parties to ensure that they use a self-consistent prefix.
docs

[ptheywood]

Configuring individual examples from the individual example directory, has the following advanced grouped GUI entry, this is due to where options / cache variables are mostly set (the root CMakeLists.txt).

CUDA_ARCH doesn't appear eitherm thgouh that might not be a problem longer term.

This is a little complicated though, as to what options should be available in this case.

A lot of options e.g. python/tests/exampeles aren't relevant, but other options are:

• FLAMEGPU_ENABLE_NVTX
• FLAMEGPU_EXPORT_RTC_SOURCES
• FLAMEGPU_CURAND_ENGINE
• FLAMEGPU_NVCC_THREADS
• `FLAMEGPU_ENABLE_NVCC_THREADS
• CUDA_ARCH (for now)

FLAMEGPU_BUILD_API_DOCUMENTATION should probably be off by default? though its not part of all so maybe just advanced?

I guess these options should be deinfed in src/CMakeLists.txt or cmake/common.cmake or their own cmake file?. It's a little bit chicken and egg, as they're internal subprojects, external projects which add_subdirectory() the main flamegpu repo will get them already (i.e. standalone).

[ptheywood]

CMake Vis build failures due to SEATBELTS checking in the vis repo, in a function with the same name as in the main repo so it conflicts? Unsure why manylinux vis is OK with that though.

Windows has a few extra mark_as_advanced needed

---

CMake macros and functions always have global scope, so like the options / cache variables we need to prefix them to avoid collisions. Duplicated code within the vis repo will want a its own versions that set relevant settings there.

Should probably add global include guards all over the shop too.

There's not a strong convention within CMake Modules (files) to have a particular naming pattern, but they now (internally) try to prefix functions with the module name, with a Upper Camel module name, then upper Snake case for function names?

https://gitlab.kitware.com/cmake/cmake/-/commit/fa5055eb565666d14231a4abf335591b69bcd14d

[ptheywood]

Testing in Windows was worth it as I had missed an important line of CMake when copying the dlls.

Vis builds tested on python and windows, vis and non vis on linux have been tested.

Also updated the freshly merged CTest logic in, making sure things are marked as advanced etc too.

---

This is now ready for review, but prior to merge does need the vis hash updating after FLAMEGPU/FLAMEGPU2-visualiser#110 has been merged.

A fresh build directory / configure is probably worthwhile if you want to see the correct state of the CMake UI after this change.

It might actually be worth prefixing compile time defines with FLAMEGPU_ too, given they are global scope, and VISUALISATION is required for use in model code. Can't provide a constexpr instead, as the API doesn't exist if VISUALISATION is off (and changing that is a load of faff).

[Robadob]

Changes seem thorough and worthwhile.

Nothing obvious stands out as missing, I'm not in a position to do a local build but CI seems happy.

[ptheywood]

I've now also prefixed the preprocessor macros with FLAMEGPU, to avoid windows.h MAX style issues.

I've also made the preproc definitions CMake target public where they are required in flamegpu headers (so they match the static lib, even if the user would attempt to redefine them).

And also made a minor fix to CMAKE_CUDA_ARCHITECTURES, as in the vis repo.

Commits are still individual for now, but will manually squash once the vis hash is finalised.

I've tested builds locally with all the modified values having different options, with both test suites passing in both cases (thoguh I didn't explicitly check behaviour). pyflamegpu.SEATBELTS and pyflamegpue.VISUALISATION were checked however both ON and OFF.

[ptheywood]

This should be in a state to be merged once CI passes, however the telemetyr PR has slightly broken python testing, when running the tests on ../tests/swig not ../tests/swig/python, which worked before. Probably because of conftest.py somehow?

python3 -m pytest ../tests/swig/
========================================================================================================= test session starts ==========================================================================================================
platform linux -- Python 3.10.6, pytest-7.2.0, pluggy-1.0.0
rootdir: /home/ptheywood/code/flamegpu/FLAMEGPU2
collected 659 items                                                                                                                                                                                                                    

../tests/swig/python/test_version.py .                                                                                                                                                                                           [  0%]
../tests/swig/python/codegen/test_codegen.py ..................................................                                                                                                                                  [  7%]
../tests/swig/python/codegen/test_codegen_integration.py .

It just stops there rather than progressing, with no errors etc.

Running via python3 -m pytest ../tests/swig/python/ behaves correctly.