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 22 forks source link

Environment Directed Graph IO #1126

Open Robadob opened 1 year ago

Robadob commented 1 year ago

Environment directed graphs, introduced by #1089, are currently only import/export-able via dedicated HostAPI functions.

They are not currently present in existing IO methods which encapsulate a whole model:

Adding support may be necessary for simulation checkpointing #806

DavidFletcherShef commented 1 month ago

In addition to model state and logging I cannot see a straightforward way to access the directed graph data for visualisation. Having a newLineSketch object generated direct from the graph data would be really useful. Very happy to be pointed in the right direction if I've missed a simple way of doing this!

Robadob commented 1 month ago

Very happy to be pointed in the right direction if I've missed a simple way of doing this!

No you haven't missed anything.

It's not something we considered, in the (closed-source) project we rendered our graph it was done by hand based on the same data-structure we built the graph from.

image

DavidFletcherShef commented 1 month ago

Thanks for the code sample, I'll do something based on that.

DavidFletcherShef commented 1 month ago

I've now run the new visualisation code from commit f3f6b0d - it's all working well.

A couple of things on the documentation:

Minor point, the docs show this - which switches from m_vis to visualisation, that could catch some one out:

flamegpu::visualiser::ModelVis m_vis = cudaSimulation.getVisualisation();
// Mark and configure graph "mygraph" for visualisation
flamegpu::visualiser::EnvironmentGraphVis g = visualisation.addGraph("mygraph");

More important, and definitely caught me out - the environmental directed graph will only plot if you include simulation.initialise(argc, argv); after setting up the visualisation. The reset of my simulation and visualisation worked without that so I've never included it. Without it the graph isn't plotted but there's no errors given, so it took me a while to realise it was needed.

Thanks for getting this new code in place - it's much easier than my previous approach.

Robadob commented 1 month ago

Thanks for the feedback.

Minor point, the docs show this - which switches from m_vis to visualisation, that could catch some one out:

That's at typo from moving the example code from where I'd been working, I'll fix that straight away.

the environmental directed graph will only plot if you include simulation.initialise(argc, argv); after setting up the visualisation.

Hmm, I had understood that environment directed graphs' vertices/edges do not exist at model definition time, as they must be initialised during an init function or similar (e.g. if you update the graph during the simulation, the visualisation should be regenerated). It's possible I overlooked something and need to add another trigger to update the visualisation, I will review.

Edit: I've reviewed the userguide, I think this behaviour is correct. Are you sure that your graph has been initialised with vertex/edge data prior to calling simulation.initialise(argc, argv);? Or is this distinction not clear and worth documenting?

DavidFletcherShef commented 1 month ago

I'm initialising the graph using graph.importGraph("../src/graph.json"); within a function FLAMEGPU_INIT_FUNCTION(InitGraph). That definitely reads the data ok as it's used in the simulation. But the graph wasn't in the visualisation without adding the simulation.initialise(argc, argv); after setting up the visualisation. Maybe it's about when the INIT function is called?

The code is here so you can see what's it was, around line 427.

Robadob commented 1 month ago

That's weird, I don't think explicitly calling initialise() should be required. The call to simulate() should do that automatically. I'll make a note to debug it over the weekend.

Thanks for clarifying and sharing the code.

Robadob commented 1 month ago

@DavidFletcherShef

I've now tracked down and resolved that bug, I expect @ptheywood will approve/merge it Monday if not sooner.

https://github.com/FLAMEGPU/FLAMEGPU2/pull/1243

DavidFletcherShef commented 1 month ago

Fantastic, thanks for looking at this.