FLAMEGPU2 icon indicating copy to clipboard operation
FLAMEGPU2 copied to clipboard

Console warning/notice when SEATBELTS is enabled.

Open Robadob opened this issue 3 years ago • 3 comments

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.

Robadob avatar May 11 '21 17:05 Robadob

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.

mondus avatar Oct 27 '22 12:10 mondus

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).

ptheywood avatar Oct 27 '22 13:10 ptheywood

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.

Robadob avatar Oct 27 '22 13:10 Robadob