materialsproject / jobflow

jobflow is a library for writing computational workflows.
https://materialsproject.github.io/jobflow
Other
90 stars 24 forks source link

BUG: to_mermaid #349

Closed JaGeo closed 1 year ago

JaGeo commented 1 year ago

Describe the bug I tried to run the following code.

from atomate2.vasp.flows.elastic import ElasticRelaxMaker
from jobflow.utils.graph import to_mermaid
from pymatgen.core.structure import Structure

flow=ElasticRelaxMaker().make(Structure.from_file("SbSe/POSCAR"))

print(to_mermaid(flow))

It fails with:

AttributeError                            Traceback (most recent call last)
Cell In[3], line 7
      3 from pymatgen.core.structure import Structure
      5 flow=ElasticRelaxMaker().make(Structure.from_file("SbSe/POSCAR"))
----> 7 print(to_mermaid(flow))

File /hpc-user/jgeorge/PycharmProjects/2023_06_09_Debug_Phonon_kpath/jobflow/src/jobflow/utils/graph.py:249, in to_mermaid(flow, show_flow_boxes)
    246             if indent_level != 1:
    247                 lines.append(f"{prefix}{job.uuid}({job.name})")
--> 249 add_subgraph(flow)
    251 return "\n".join(lines)

File /hpc-user/jgeorge/PycharmProjects/2023_06_09_Debug_Phonon_kpath/jobflow/src/jobflow/utils/graph.py:236, in to_mermaid.<locals>.add_subgraph(nested_flow, indent_level)
JaGeo commented 1 year ago

I know that it is probably not intended to just visualize one job but I might not be the only one that by accident passes a single job instead of a Flow.

JaGeo commented 1 year ago

I am closing it as the documentation clearly says it should be a Flow.

davidwaroquiers commented 1 year ago

A quick fix would just be to check if it's a single Job, and if so, just wrap it with a Flow and continue just like that.

JaGeo commented 1 year ago

A quick fix would just be to check if it's a single Job, and if so, just wrap it with a Flow and continue just like that.

This would work if a single job in a Flow would create an output. Maybe, there is a real bug then:

from jobflow.utils.graph import to_mermaid
from jobflow import job, Flow
@job
def add(a, b):
    return a + b
add_first = add(1, 2)
my_flow = Flow(jobs=[add_first])
graph_source = to_mermaid(my_flow)
print(graph_source)

produces

flowchart TD
JaGeo commented 1 year ago

If we remove the following line, it should work: https://github.com/materialsproject/jobflow/blob/09ffa16026b5682f7947045fedf6376ce7276d7f/src/jobflow/utils/graph.py#L246

I am just wondering if there is an intention beyond omitting to visualize just one job.

utf commented 1 year ago

Thanks for the discussion. Closed with #350

mkhorton commented 1 year ago

Adding a note that mermaid syntax supports subgraph, if it'd be useful we could show nested Flows as subgraphs? Would this be helpful?

mkhorton commented 1 year ago

Actually nevermind, I see this was already implemented(!) Very cool