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

Fix networks #155

Open
wants to merge 51 commits into
base: main
Choose a base branch
from
Open

Fix networks #155

wants to merge 51 commits into from

Conversation

IAlibay
Copy link
Member

@IAlibay IAlibay commented Oct 5, 2024

Supplements #144

This needs:

  • Fixing (there's an issue with connected networks)
  • Testing
    • Please add tests for every method w/ pytest. There's too much going on here and half the things are probably broken, so we need to test them.
  • Documentation
  • Add documentation that tells users what they should be doing with this script.

RiesBen and others added 30 commits September 3, 2024 16:44
removing copy past artefact :)
# build alchemical sub-networks
g = ligand_network.graph
all_edges = list(ligand_network.edges)
sub_network_nodes = list(nx.connected_components(g))
Copy link
Member Author

Choose a reason for hiding this comment

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

This is failing because it's a multidigraph I think - we can just create a digraph from g and it should work

Copy link
Contributor

Choose a reason for hiding this comment

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

Now first creating an undirected graph from g

@hannahbaumann hannahbaumann self-assigned this Oct 7, 2024
Copy link
Contributor

@hannahbaumann hannahbaumann left a comment

Choose a reason for hiding this comment

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

Will start addressing the comments I've raised.


transform_list.append(t_list[0])

return AlchemicalNetwork(transform_list)
Copy link
Contributor

Choose a reason for hiding this comment

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

The AlchemicalNetwork may not have "complete" cycles, i.e. could only have the solvent or complex leg. When generating the LigandNetwork from this, this information gets lost and broken networks are not recognized.

Copy link
Contributor

Choose a reason for hiding this comment

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

This was fixed in commit 8f7131f

Copy link
Contributor

Choose a reason for hiding this comment

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

If instead the user should remove result .json files from edges where only one leg finished, these two lines can be removed, though at least a warning should be thrown.

stateB = ChemicalSystem.from_dict(units[0]["inputs"]["stateB"])
ligmap = LigandAtomMapping.from_dict(units[0]["inputs"]["ligandmapping"])

if any([isinstance(comp, gufe.ProteinComponent) for comp in stateA.components.values()]):
Copy link
Contributor

Choose a reason for hiding this comment

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

ToDo: Test if this would still work for files that have been cleaned up, i.e. where components have partially been removed.

# build alchemical sub-networks
g = ligand_network.graph
all_edges = list(ligand_network.edges)
sub_network_nodes = list(nx.connected_components(g))
Copy link
Contributor

Choose a reason for hiding this comment

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

Now first creating an undirected graph from g

@hannahbaumann
Copy link
Contributor

hannahbaumann commented Oct 8, 2024

ToDo:

  • Write out ligand_network.graphml file or the "new" network (old edges + new edges), old edges that had failed have the annotation "failed" = True

@hannahbaumann
Copy link
Contributor

hannahbaumann commented Oct 9, 2024

ToDo

  • We should probably add the same checking for ProtocolUnitFailure s as in the extract_results.py script in case users haven't deleted failed result .json files.

@IAlibay IAlibay self-assigned this Oct 10, 2024
return ru


def get_transformation_alternate(
Copy link
Member Author

Choose a reason for hiding this comment

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

From today: we could possibly get solvent/complex from the names.

for transform in taped_alchemical_network.edges:
transform.dump(transforms_dir / f"{transform.name}.json")

full_ligand_network = get_full_ligand_network(input_ligand_network, res_ligand_network, network_connections)
Copy link
Member Author

Choose a reason for hiding this comment

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

Change this to be only the additional edges.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Addressed in commit 31c2ac1

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.

Fix network: Save out ligand_network.graphml file with only new edges
3 participants