-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Telemetry #987
Telemetry #987
Conversation
ee38875
to
fdd88c2
Compare
188df1d
to
96f8582
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- You haven't updated CI to set
FLAMEGPU_SHARE_USAGE_STATISTICS
when building release wheels. - The spurious (empty) file
examples/boids_bruteforce/-X
should be deleted.
Think I've left all my comments now.
tests/helpers/main.cu
Outdated
// Time the cuda agent model initialisation, to check it creates the context. | ||
flamegpu::tests::timeCUDASimulationContextCreationTest(); | ||
// google test doesnt have a verbose mode so check for verbosity flag for telemetry output | ||
bool verbose = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just always print this info, there's no help output either so no one running the test suite will know this exists anyway, and it won't be triggered by CTest.
It's on by default, so doesn't need explicitly setting (original was going to be opt-in, but Paul changed his mind) |
…er than env var from python init.py
CUDACC versions macros are only available when nvcc is the host compiler, so must be done in a .cu file, hence the duplication.
3b714ef
to
d4c7db6
Compare
Test suite results being phoned home have been removed for now, as swig needs to wrap the methods to make the call out, but they are intentionally hidden so users can't send them. In the interest of rc-1, this has been removed for now to get other PRs merged / directory restructuring done, it may be re-added later if time allows, which will be an ABI break / switch back to a namespace + detail namespace + swig renaming + explicit not ignoring of that detail namespace... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now approved after a discussion / couple of fixes w/ paul.
Will merge once CI passes.
Adds telemetry to the software via the TelemetryDeck web service. This is an important feature for evidencing usage of the software. Without evidence of impact the software will not be able to apply for funding to support its continued development. After consultation with the RSE community the software will be opt-in to enable sharing of usage data. There are various way in which telemetry can be enabled and various environment variables which can be set. Please read the docs PR for a summary of these
Main features of this branch;
FLAMEGPU_SHARE_USAGE_STATISTICS
. If the value is false a status message is given to users to encourage them to enable it to support the flame gpu software.FLAMEGPU_SHARE_USAGE_STATISTICS
global variables to enable telemetry. The default value of simulation telemetry is ON.Note to devs: Set an environment variable
FLAMEGPU_TELEMETRY_TEST_MODE
from this point onward so any telemetry from us goes in test mode in TelemetryDeck