ExplicitNetworkGenerator missing unconnected nodes

[atravitz]

Describe the bug

The method ExplicitNetworkGenerator.generate_network_from_names(components, names) only includes nodes (components) that are connected by edges. i.e., no unconnected/floating nodes are included. This is in contrast to the current openfe network generating behavior.

I noticed this when unit tests for this openfe PR failed, because ligand_network_planning.generate_network_from_names() is being switched to use konnektor's ExplicitNetworkGenerator implementation.

To Reproduce

This Konnektor unit test demonstrates this.

Expected behavior

@IAlibay I believe we should change the Konnektor implementation to match the current openfe behavior. However, I'd like to know if this was intentionally done by @RiesBen as part of Konnektor's design.

[RiesBen]

@atravitz
hi, jap I wanted that behavior, as I wanted konnektor to not return disconnected networks by default. But maybe simply throwing an error would have been wiser?

[IAlibay]

I think this needs discussion, but disconnected networks is something we will likely want under certain circumstances. I remember the original behaviour was somewhat of an intentional design decision (although I had asked that it threw at least an error to let folks know the network was disconnected).

[atravitz]

In gufe, a single AlchemicalNetwork can contain multiple subnetworks, which can be represented as AlchemicalNetworks themselves.

As of now, it seems that konnektor enforces that all networks are connected, and so we could treat a disconnected network as as an iterable of networks. This way, konnektor's requirements would be more strict but still within gufe's requirements.

[IAlibay]

Just picking on the main case where this is failing right now: generating a network from names.

If I pass a list of name tuples that define every edge in a network, I expect to get a network that has all these edges, not just a subset of that network that is connected.

What is our proposed behaviour in that case? That it just fails? That it returns an iterable of networks (in that case why not just concatenate them all together)? That is silently returns only the connected parts (which would be a bad idea imho)?

[atravitz]

@atravitz hi, jap I wanted that behavior, as I wanted konnektor to not return disconnected networks by default. But maybe simply throwing an error would have been wiser?

@RiesBen - what is your reason for wanting konnektor to forbid disconnected networks? where would you expect this to cause issues?

[RiesBen]

Hej @atravitz,
I think it was a condition, that I took for the graph theory fundament.

Reason 1: I think all Konnektor implemented Algorithms are required to yield connected graphs, otherwise something went wrong, so I used this as a proof of concept check and it made working with networks for me more consistent.

Reason 2: A relative FE network that is not connected looses at least one or more ligands in the final ranking. So in the relative FE case, it is required to have a connected graph to start with.

But is that really required? hm ... a question of handling. If you would like to exclude certain edges, you could filter networks for only edges you want to have, after generating them...

hope this helps? :)