OpenFreeEnergy / konnektor

Algorithms for various Network Layouts and Tooling for planning FE Calculations
https://konnektor.readthedocs.io/
MIT License
14 stars 3 forks source link

ExplicitNetworkGenerator missing unconnected nodes #92

Open atravitz opened 2 weeks ago

atravitz commented 2 weeks ago

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 commented 2 weeks ago

@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 commented 2 weeks ago

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).