FLAMEGPU2 icon indicating copy to clipboard operation
FLAMEGPU2 copied to clipboard

Environmental variables for Jitify/RTC

Open ptheywood opened this issue 4 years ago • 7 comments

Currently 2 environment variables are required to jitify/rtc compilation:

  • CUDA_PATH pointing to the root of the relevant CUDA version
  • FLAMEGPU2_INC_DIR pointint to flamegpu2_dev/include

This could be improved by:

  • [x] also attempting other common environmental variables (CUDA_HOME/CUDA_ROOT are semi-common alternatives to CUDA_PATH)

  • [ ] fallback to trying the baked in path to the CUDA version used to compile the executable (if running on the same machine)

  • [ ] try he location of include from compile time (abs path, and / or relative to executable expected location)

  • [ ] Possibly pack the include headers into the executable? (this seems daft)

  • [ ] set FLAMEGPU2_ROOT instead, and infer /include from that.

  • Readme statment about CUDA_PATH being set by the installer is untrue for linux and also for windows when using the silent installer

This needs to be

  • [ ] Clearly documented

The current implementation (fbf2c8e) also doesn't check that the value of FLAMEGPU2_INC_DIR is valid or correct, leading to misleading errors, rather than an appropraite exception.

  • [x] Check values in environment variables before using them, with appropriate exceptions if invalid
    • [x] Check that FLAMEGPU2_INC_DIR is a valid directory, with atleast one flamegpu2 header file.
      • [x] Ideally this should probably also do a version check once we have solid versioning built in, so that the "same" headers are used as the library was built with.
    • [x] Check that CUDA_PATH is valid (look for bin/nvcc)
      • ~[ ] Ideally this should also check that the version of CUDA matches the version the library was compiled with? (Not sure if this is a hard requirement or not, or if it would work so long as the version is newer?)~
      • [ ] Given the static library doesn't build any RTC related cuda code, does not need to be the same version, but we should probably check it is a supported CUDA version (which Cmake currently deals with for building the executables). I.e. check the version of NVCC on the CUDA_path is atleast 9.X (or in practice we can move to 10.0+?)

When including the FLAMEGPU2 repo via cmake in an individual example project, the include dir is not found automatically, requiring the env variable to be set. The env variable being set globally is also a potential issue, i.e. if you have 2 examples each using a different version of the core F2 on the same machine.

  what():  /home/ptheywood/code/flamegpu/FLAMEGPU2-circles-benchmark/build/_deps/flamegpu2-src/src/flamegpu/util/JitifyCache.cu(106): Error compiling runtime agent function: Unable to automatically determine include directory and FLAMEGPU2_INC_DIR environment variable does not exist, in JitifyCache::buildProgram().
  • [ ] Fix automatic search for the include directory for individual examples, when FLAMEGPU2_INC_DIR is not defined.
  • [ ] Figure out what to do for individual examples if FLAMEGPU2_INC_DIR is defined but is wrong. maybe fallback to checking locally?
  • [ ] Rename FLAMEGPU2_INC_DIR FLAMEGPU_INC_DIR, as it will not always be FLAMEGPU 2.x.y?

ptheywood avatar May 06 '20 18:05 ptheywood

Possibly pack the include headers into the executable? (this seems daft)

It's certainly unusual, however current headers dir is 503kb, size of the smallest debug executable on windows is 20mb.

Bearing in mind, we plan to reduce the size of header dir further in future, it's not the most insane idea.

It's also the only portable method, assuming no weird machines without write access. Although jitify does just load the headers from disk, so we could pass them as string in the jitify (filename)\n<file data> format.

Robadob avatar May 06 '20 18:05 Robadob

The recent patch searches relative to the current working directory, rather than the current binary's directory, so calling from outside can lead to the path not being found. Both would probably be a wise choice.

ptheywood avatar Oct 19 '20 17:10 ptheywood

That's a good shout, I didn't consider it tbh. I'd just need to check each platform's way of getting binary location. Trivial addition.

https://stackoverflow.com/a/1528493/1646387

Robadob avatar Oct 19 '20 17:10 Robadob

Some of this should also be conisdered pre binary release #514

ptheywood avatar Jul 21 '21 10:07 ptheywood

Worth noting, the Python wheel now includes the RTC headers. Though C RTC binaries don't.

Robadob avatar Jul 21 '21 10:07 Robadob

support FLAMEGPU2_INC_DIR (deprecated but works, lwoer priotiy) and FLAMEGPU_INC_DIR

ptheywood avatar Jul 21 '21 12:07 ptheywood

#1074 Adds GLM headers to Python wheels (as this seems compliant with the license as that's packaged too) and setups up GLM_INC_DIR environment variable to override path.

Robadob avatar Jul 05 '23 15:07 Robadob