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

Script to plan RBFE alchemical networks #31

Merged
merged 15 commits into from
Jun 12, 2024
Merged

Script to plan RBFE alchemical networks #31

merged 15 commits into from
Jun 12, 2024

Conversation

hannahbaumann
Copy link
Contributor

@hannahbaumann hannahbaumann commented May 28, 2024

Input submission checklist

  • created a directory for each system based on the template directory
  • named and placed the directory with the following pattern: input_structures/prepared_structures/<set_name>/<system_name>

For each submitted directory:

  • filled in system preparation details in the PREPARATION_DETAILS.md file
  • added a PDB file with the protein named protein.pdb
  • removed any cofactors from the PDB file and placed them in cofactors.sdf
  • kept cystallographic waters and metals in the PDB file
  • tested the PDB and, if available, cofactors.sdf file using the input validation script
  • copied over the ligands SDF file to a file named ligands.sdf
  • copied over the edges CSV file to a file named edges.csv

Summary of changes

Licensing agreement

  • I agree to release these inputs under the combined MIT and CCBY SA 4.0 licenses of this repository

@pep8speaks
Copy link

pep8speaks commented May 28, 2024

Hello @hannahbaumann! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 10:80: E501 line too long (88 > 79 characters)
Line 27:80: E501 line too long (82 > 79 characters)
Line 70:80: E501 line too long (88 > 79 characters)
Line 75:80: E501 line too long (85 > 79 characters)
Line 208:80: E501 line too long (85 > 79 characters)
Line 219:80: E501 line too long (80 > 79 characters)
Line 220:80: E501 line too long (80 > 79 characters)
Line 251:80: E501 line too long (81 > 79 characters)

Line 50:80: E501 line too long (83 > 79 characters)
Line 56:80: E501 line too long (84 > 79 characters)
Line 56:85: W291 trailing whitespace
Line 61:80: E501 line too long (82 > 79 characters)
Line 62:80: E501 line too long (82 > 79 characters)
Line 75:80: E501 line too long (97 > 79 characters)
Line 76:80: E501 line too long (82 > 79 characters)
Line 78:80: E501 line too long (85 > 79 characters)

Comment last updated at 2024-06-11 08:59:25 UTC

@hannahbaumann hannahbaumann linked an issue May 29, 2024 that may be closed by this pull request
get_settings_charge_changes()


# Test that the number of transformation files for solvent and complex are equal
Copy link
Member

Choose a reason for hiding this comment

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

Needed tests:

  • Serialize LigandNetwork; check it's there and that the network has the files we need it to have
    • Check the score annotations for at least one edge
    • Check the smcs have partial charges for at least one edge
  • Check the type of files being created (so is there a json and how many)
  • Check the settings in the JSON files (i.e. we should know that there are N json files with charge changes and N without)

Copy link
Member

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

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

Just reminding myself that I still have to look at the residue name stuff!

ligand_network = openfe.ligand_network_planning.generate_lomap_network(
molecules=smcs, mappers=mapper, scorer=scorer)
# Raise an error if the network is not connected
if ligand_network.is_connected() is False:
Copy link
Member

Choose a reason for hiding this comment

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

This is purely a "comment on python programming style" not something you need to take any action on.

Just wanted to say that folks generally expect "if not X" rather than "if X is False".

"""
offmol = smc.to_openff()
with toolkit_registry_manager(amber_rdkit):
offmol.assign_partial_charges(partial_charge_method="am1bcc")
Copy link
Member

Choose a reason for hiding this comment

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

here we should be enforcing the input conformer when assigning partial charges

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed this!

openfe.LigandNetwork
The Lomap generated LigandNetwork.
"""
mapper = kartograf.KartografAtomMapper(atom_map_hydrogens=True)
Copy link
Member

Choose a reason for hiding this comment

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

from today's call - should we pin the defaults?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this!

@RiesBen RiesBen self-requested a review June 11, 2024 09:16
Copy link
Contributor

@RiesBen RiesBen left a comment

Choose a reason for hiding this comment

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

Looks Good to me!

I think for the future, we should make more use of the alchemical network planners, or develop them further, sucht they are more useful. But this is not an issue for this time. Still would like to get feedback on those. @IAlibay @hannahbaumann

@IAlibay
Copy link
Member

IAlibay commented Jun 12, 2024

Looks Good to me!

I think for the future, we should make more use of the alchemical network planners, or develop them further, sucht they are more useful. But this is not an issue for this time. Still would like to get feedback on those. @IAlibay @hannahbaumann

Yeah - this is definitely at the top of my mind as we plan out our next pieces of work.

@IAlibay IAlibay merged commit 591ba0a into main Jun 12, 2024
1 check passed
@IAlibay IAlibay deleted the run_script branch June 12, 2024 09:02
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.

Create RBFE simulation run script
4 participants