JuliaParallel / Dagger.jl

A framework for out-of-core and parallel execution
Other
610 stars 66 forks source link

Fix write dag #531

Open SmalRat opened 4 weeks ago

SmalRat commented 4 weeks ago

Fixes for write_dag() and show_logs() functions. More details on #512 .

jpsamaroo commented 4 weeks ago

Awesome stuff! I'm happy to accept this as-is if you'd like, but there are also a few things we can improve if you want to tackle them:

Let me know if you want this merged now or if you want to try to tackle some of these, I can always add these to the TODO list :smile:

SmalRat commented 3 weeks ago
  • Add a render_logs alternative (similar to render_logs(logs, :graphviz)) that returns a renderable GraphViz object (and equivalently, we could implement show_logs(logs, :graphviz) like you've done here for rendering directly to an IO)
  • Consider adding a save_logs function that drives Cairo to save renderings to a file, or make render_logs(path::String, ...) somehow dispatch to do this (maybe via FileIO.jl)
  • Document (under docs/src/logging-visualization.md) this newly-supported renderer, so that users can more easily discover it

Let me know if you want this merged now or if you want to try to tackle some of these, I can always add these to the TODO list 😄

Looks like it is not a problem to add this. I will do that soon

SmalRat commented 6 days ago

Hi!

There is an issue with implementing these features:

  • Add a render_logs alternative (similar to render_logs(logs, :graphviz)) that returns a renderable GraphViz object (and equivalently, we could implement show_logs(logs, :graphviz) like you've done here for rendering directly to an IO)

  • Consider adding a save_logs function that drives Cairo to save renderings to a file, or make render_logs(path::String, ...) somehow dispatch to do this (maybe via FileIO.jl)

It seems that GraphVizSimpleExt, which I am working on, was intended to be used when GraphViz is not imported (with GraphVizExt for the opposite case). Am I correct in this assumption?

If that is the case, GraphVizSimpleExt can not return a renderable GraphViz object, which we want from the render_logs function and which is to be used in save_logs.