Skip to content

[WIP] Add a notebook to repair broken networks - #321

Open
hannahbaumann wants to merge 2 commits into
mainfrom
fix-broken-networks
Open

[WIP] Add a notebook to repair broken networks#321
hannahbaumann wants to merge 2 commits into
mainfrom
fix-broken-networks

Conversation

@hannahbaumann

Copy link
Copy Markdown
Contributor

This is a first draft for a notebook to show how to fix broken networks by ligand names or lomap scores.
The tutorial reuses parts of the fixing networks script from the industry benchmarking project (https://github.com/OpenFreeEnergy/IndustryBenchmarks2024/blob/main/industry_benchmarks/utils/fix_networks.py).

This tutorial also shows that the MSTConcatenator in Konnektor is currently not acting as intended, but creates too many additional edges, see here OpenFreeEnergy/konnektor#263.

This notebook is reusing old results (the same that are used in the showcase notebook), and uses the industry benchmark lomap network as the input network, however, three of the old result edges are not present in that ligand network.

LLM disclosure: This notebook was written together with Claude (opus 4.8).

@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions

Copy link
Copy Markdown

Colab 👈 Launch a Colab session on branch fix-broken-networks

@@ -0,0 +1,749 @@
{

@hannahbaumann hannahbaumann Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line #1.    def _normalise(name: str) -> str:

This is a bit of an annoying thing where the old results have the lig_ prefix, but the industry benchmark network does not. We could also generate new results instead?


Reply via ReviewNB

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah can we simplify or hide any of this to keep the notebook really focused on the repair? Can we mock a broken network just from pulling the results and maybe not use the industry benchmark network?

@@ -0,0 +1,749 @@
{

@hannahbaumann hannahbaumann Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line #24.    def decompose_network(network):

This function could eventually also live in the konnektor network tools, see OpenFreeEnergy/konnektor#261


Reply via ReviewNB

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice yeah this would make sense to be in konnektor if it already has the merging and concat functions.

@@ -0,0 +1,749 @@
{

@hannahbaumann hannahbaumann Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit annoying, not sure if it's confusing in the notebook and better to generate a new zenodo entry without these edges?


Reply via ReviewNB

@@ -0,0 +1,749 @@
{

@hannahbaumann hannahbaumann Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MSTConcatenator adds n_connecting edges between each fragment with each other one, so it create way too many edges, see OpenFreeEnergy/konnektor#263


Reply via ReviewNB

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah if we could simplify this example to a simple break between two subnetworks while we fix this that would be great.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if instead we should wait on merging/advertising this notebook until we have made the fix in Konnektor and cut a new release, otherwise people may start using this and report those problems.

@@ -0,0 +1,624 @@
{

@jthorton jthorton Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about if we drew the disconnected network from the results we could use cinnabar (this would mean we need to pull and combine the results - might be overly complicated) or we could just write a function to do the same plot and show the reader the broken and planned networks side by side?


Reply via ReviewNB

@@ -0,0 +1,624 @@
{

@jthorton jthorton Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line #1.    def repair_by_names(completed, ligands, mapper, names):

As this is one of the two main functions of the notebook the users will want to copy lets add a full docstring with typing to make it clear what the inputs are.


Reply via ReviewNB

@@ -0,0 +1,624 @@
{

@jthorton jthorton Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line #1.    def repair_by_score(fragments, mapper, scorer, n_connecting_edges=1):

Same as above lets add a full docstring with type hints and explain what the function is trying to do.


Reply via ReviewNB

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

Successfully merging this pull request may close these issues.

2 participants