search

cyclus / cymetric

http://fuelcycle.org/user/cymetric/index.html
Other
5 stars 19 forks source link

Adding region and institution hierarchy to graphs/subgraphs #197

Closed stompsjo closed 2 months ago

stompsjo commented 2 months ago

Hello all! I noticed that the default cymetric.flow_graph was not giving me quite the flexibility I wanted and thought I would offer this PR in case anyone else was interested. Here's a summary of what these changes do:

  1. All unique agents in a Cyclus output will be plotted in the flow graph. That means that if you have five PWRs, there will be five PWR nodes in your graphviz output. This is on account of looping over the IDs produced by evaler.eval("AgentEntry") rather than with evaler.eval("AgentEntry")["Prototype"].tolist(). Maybe we could include another input to flow_graph that disables this functionality.
  2. Regions and institutions will be plotted as subgraphs, rather than as disconnected nodes. This requires using the dot.subgraph(name="cluster_") syntax, but I think it results in a more organized graph (see example below). Again, all unique regions and institutes in your simulation will be plotted here. I took the liberty of setting some default visualization styles (dotted squares for regions, gray-filled squares for institutions) but I could imagine us supporting flexibility for this feature in the future.
  3. Using strict=True in the graph ensures that only unique edges are plotted, i.e. if multiple transactions take place between the same two nodes, only one of those edges is plotted. I don't recall if this was a concern before, but I think it might keep things safe from producing vast spaghetti plots.

The default engine/solver of graphviz does a good job of organizing the rendered plot, but I have noticed more edge overlap when using this subgraph structure. I have attached an example plot using the tutorial exercise from fuelcycle.org. You can recreate this image by running cyclus example.xml and then the following code:

import cymetric as cym

db = cym.dbopen('cyclus.sqlite')
evaler = cym.Evaluator(db, write=False)
dot = cym.flow_graph(evaler)
dot.render('example', format='png', cleanup=False)

example

github-actions[bot] commented 2 months ago

Build Status Report

Build FROM cycamore_20.04_apt/cycamore:latest - Success :white_check_mark: Build FROM cycamore_20.04_apt/cycamore:stable - Success :white_check_mark: Build FROM cycamore_20.04_conda/cycamore:latest - Success :white_check_mark: Build FROM cycamore_20.04_conda/cycamore:stable - Success :white_check_mark: Build FROM cycamore_22.04_apt/cycamore:latest - Success :white_check_mark: Build FROM cycamore_22.04_apt/cycamore:stable - Success :white_check_mark: Build FROM cycamore_22.04_conda/cycamore:latest - Success :white_check_mark: Build FROM cycamore_22.04_conda/cycamore:stable - Success :white_check_mark:

gonuke commented 2 months ago

Thanks @stompsjo for this contributions. Can you please add an entry to the Changelog? I also wonder why this fails on conda? That may be a question for @bennibbelink

bennibbelink commented 2 months ago

The errors occuring are: ValueError: numpy.dtype size changed, may indicate binary incompatibility. Expected 96 from C header, got 88 from PyObject

Initially I thought it might be related to the recent switch to mambaforge, however the error is occurring on the stable version as well (which still uses miniconda). This stackoverflow post suggests we may need to pin the numpy version for now... I'm guessing just for as long as it takes pandas to catch up and release a version compatible with numpy==2.0.0

EDIT: Pandas 2.2.2 should be compatible with numpy 2.0. Looking into this more

bennibbelink commented 2 months ago

To the best of my knowledge this is the dependency situation:

An issue was made last week in the PyTables repo regarding this.

Until that issue is resolved... if we want CI to pass I think the easiest fix is to pin numpy<2.0.0 in Cymetric's pyproject.toml. @gonuke does this seem like a reasonable temporary fix?

gonuke commented 2 months ago

That's fine as long as we have a way to remember to check on this as the compatibility progresses

bennibbelink commented 2 months ago

Sounds good. I will make a PR with the temporary fix, as well as an open issue to check on pytables compatibility