Skip to content
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

BUG: to_mermaid #349

Closed
JaGeo opened this issue Jun 14, 2023 · 8 comments
Closed

BUG: to_mermaid #349

JaGeo opened this issue Jun 14, 2023 · 8 comments

Comments

@JaGeo
Copy link
Member

JaGeo commented Jun 14, 2023

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
Copy link
Member Author

JaGeo commented Jun 14, 2023

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
Copy link
Member Author

JaGeo commented Jun 14, 2023

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

@JaGeo JaGeo closed this as completed Jun 14, 2023
@davidwaroquiers
Copy link
Contributor

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
Copy link
Member Author

JaGeo commented Jun 14, 2023

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 JaGeo reopened this Jun 14, 2023
@JaGeo
Copy link
Member Author

JaGeo commented Jun 14, 2023

If we remove the following line, it should work:

if indent_level != 1:

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

@utf
Copy link
Member

utf commented Jun 14, 2023

Thanks for the discussion. Closed with #350

@utf utf closed this as completed Jun 14, 2023
@mkhorton
Copy link
Member

mkhorton commented Aug 3, 2023

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
Copy link
Member

mkhorton commented Aug 3, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants