-
Notifications
You must be signed in to change notification settings - Fork 21
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
Graph Visualisation #1239
Graph Visualisation #1239
Conversation
@ptheywood |
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.
Appears to work correctly under a native linux build.
Changes look fine to me (but I've not been too thoroguh) and we have an absence of tests for vis parts of the API (not ideal, but not worth the time investment right now to address).
setXProperty()
perhaps better changed tosetVertexXProperty()
to be more explicit?
I agree, think the more explicit one is probably clearer.
Vis PR needs merging and vis updating prior to merge (as the branch will not exist forever).
I'm in favour of ignoring the cpplint CI for now, can fix independently (we should suppress the rule & namespace indent rule, but prolly fix the include warnings). Might be best to do that after this is merged, or rebase this if fixed independenltly sooner.
The manylinux failures we will need to fix in general (container must have been updated to no longer include setuptools by default, so we will need to install it). Again we can prolly ignroe that failure for now, but might be better to fix it before merge just in case there is some other issue?
Visualisation will update if the graph is changed during the simulation. Requires updates to the visualiser too.
This update pulls in support for graph visualisation and initial camera roll.
It now works, model used for testing can be found here.
2024-10-19.16-39-25.mp4
Requires update to vis repo (same branch name) to build/work.: FLAMEGPU/FLAMEGPU2-visualiser#138
Related docs PR: FLAMEGPU/FLAMEGPU2-docs#179
Cpplint has updated to v2.0.0 and introduced some nasty aggressive rules, hence lint failure (it was happy with v1.6.1 prior to me updating locally). Likewise various CI failures (windows cuda, setuptools).