Suggestion: support for MultiDiGraph

[AntonioRochaAZ]

Hello, I have a suggestion. Currently, networkx.DiGraph is used for the graph representation of flowsheets. Although adequate for probably all real flowsheets, if the standard is to be used for machine learning applications, there may be loss of information in some edge cases.

For example: "(raw)(col){bout}%1[{tout}(mix)<1(prod)]" represents a flowsheet composed of a column, both exits of which are connected to the same mixer. The DiGraph class does not allow two different edges between the same two nodes, so one of them will be deleted:

f = Flowsheet(sfiles_in="(raw)(col){bout}%1[{tout}(mix)<1(prod)]")
print(f.state.edges()) # Gives: OutEdgeView([('raw-1', 'col-1'), ('col-1', 'mix-1'), ('mix-1', 'prod-1')])
f.convert_to_sfiles()
print(f.sfiles)              # Gives: '(raw)(col){bout}(mix)(prod)'

Of course this process is not relevant in practice, but if a machine learning model is used for generating processes, which are then converted into graphs for simulation, this loss of information may sometimes become an issue during runtime.

I believe that by simply changing the references to networkx.DiGraph to networkx.MultiDiGraph solves this problem. (it may be good to analyse if this doesn't break other parts of the code/introduces edge cases of course).

[LukasSchBal]

Thanks for the suggestion! Switching to networkx.MultiDiGraph is definitely a sensible direction, but as you mentioned, it would be important to test carefully to ensure compatibility throughout the library.
If you're up for it, feel free to make the change and open a merge request! We'd be happy to review it. Of course, we will also look into it!

[AntonioRochaAZ]

I'll see what I can do :)

[LukasSchBal]

I think that using MultiGraph would require more work than simply changing nx.DiGraph to nx.MultiDiGraph as MultiDiGraph defines the edge attributes as a dict of dicts to accomodate for multiple edges whereas DiGraph uses dicts.
e.g.

• MultiDiGraph: {"edge1": {"tags": "not_next_unitop"}}
• DiGraph {"tags": "not_next_unitop"}

The code must be refactored to accomodate for this. We will not focus on this for now, but are happy to discuss and review potential changes :)

[AntonioRochaAZ]

I'll keep that in mind! I am a little busy these next few weeks but I would gladly go through the code calmly to refactor when I have the time!

[AntonioRochaAZ]

To give an update: I've started looking at the code! It appears to be simple to make the change to MultiDiGraph without breaking what already works (DiGraph structures). However, for having full working support for actual MultiDiGraphs I'll have to dive deeper to see what that entails. I think that some functions like nx_to_sfiles.dfs implicitly assume that the graph is a DiGraph (we loop on successor nodes and not out-edges, the length of which may be different in MultiDiGraph).

By the way, I've been solely looking into adaptation of the framework and haven't really thought about if the Morgan algorithm still makes sense for MultiDiGraphs. I would guess so but I am not sure.