search

FLAMEGPU / FLAMEGPU2

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

Console warning/notice when SEATBELTS is enabled. #526

Open Robadob opened 3 years ago

Robadob commented 3 years ago

Whilst discussing how we'll require multiple python packages with @ptheywood, came to recognise there would be value in highlighting to users when they are using a library build with SEATBELTS enabled, as this is intended for 'diagnostics' rather than to maximise performance.

This warning should obviously appear once per run, preferably as the first item to be output to console. Slight concern over warning fatigue, but if that's really a problem we could perhaps allow an env variable to supress the warning (assuming [some] users dev on local, run on HPC). I think this case would be better output to stderr (@ptheywood may disagree, not sure I convinced him).

@ptheywood also raised that we should highlight SEATBELTS in some form when the user requests performance metrics, e.g. to console or output file. In the case of output file, we could perhaps add it would be most appropriate to add it to the configuration block, as a value which is only ever written and never read back.

This topic obviously needs some discussion.

mondus commented 1 year ago

I'm not a fan of this. The default Python build would have seatbelts on so would result in warnings each simulation. I'd be more inclined (but not convinced) to warn that seatbelts are off (which requires changing a default setting) and having a cmake flag to disable the warning as well.

I'm not even sure any warning is needed to be honest. It is well documented.

ptheywood commented 1 year ago

A workaround could be to make it programatically accessible, which is already the case in python as pyflamegpu.SEATBELTS iirc. In C++ there's the define so its' available at preproc time (which is neccesary unfortunately due to use within method prototypes for separate builds (which is kinda grim)), but we could expose that as a static constexpr bool in the flamegpu namespace? (thought the preproc symbol of the same name will make that fun).

I agree that we should minimise warning spam. Perhaps only emit to stdout if a verbose flag is enabled (fold into #677?), and / or make it part of the timing output, or above mentioned option of emitting perf metrics to disk at the end of a run in json file or similar (including cuda ver, driver ver, GPU, CPU, etc. Everything needed so when writing stuff up after generating data you can be explicit about the configuration).

Robadob commented 1 year ago

Personally, I think we should make the warning more prominent if anything (e.g. make it print to console in yellow). If SEATBELTS=ON is what you expect to be the norm on Python, we want people to be well aware that if they're getting bad performance, that's why.

They can always pass the quiet runtime flag (or programmatically set it) if they don't care as they're developing.