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

Avoiding tleap #197

Merged
merged 4 commits into from
Nov 29, 2023
Merged

Avoiding tleap #197

merged 4 commits into from
Nov 29, 2023

Conversation

jaketanderson
Copy link
Contributor

I added a notebook to serve as a tutorial/template for setting up an APR calculation without using tleap. I chose the sugammadex-rocuronium host-guest pair for this.

@codecov
Copy link

codecov bot commented Oct 9, 2023

Codecov Report

Merging #197 (7e70d42) into master (8655108) will decrease coverage by 0.02%.
The diff coverage is n/a.

"id": "9ee91f39-6968-448d-8173-38f0c14928b5",
"metadata": {},
"source": [
"The experimental binding free energy is -8.27 kcal/mol<sup>1</sup>. To get a more accurate calculation we can decrease the step size of the attach fractions, increase the pull distance somewhat, decrease the step size between pull distances, to name a few options. Also realize that here we are only calculating the binding of rocuronium in one orientation to one face of sugammadex (primary).\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mean to increase the number of steps (or increase sampling) in the attach phase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I chose to say "decreasing the step size" because my attach fractions were given by np.arange(), but if the attach fractions were not distributed evenly it would probably be more correct to say "increasing the number of fractions."

"name": "stderr",
"output_type": "stream",
"text": [
"2023-10-09 02:10:16 PM Note: detected 128 virtual cores but NumExpr set to maximum of 64, check \"NUMEXPR_MAX_THREADS\" environment variable.\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you know a way to suppress these warning messages in Notebook? Not critical but it is a bit annoying.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I add the line os.environ['NUMEXPR_MAX_THREADS'] = 'n' on my machine to get it to shut up, but I don't know what library is dependent on numexpr and causing this message. Is this caused by a pAPRika dependency?

" constraints=HBonds,\n",
") # We recreate the OpenMM system because it now has waters\n",
"\n",
"# Set the nonbonded forces for the dummy atoms to have force constants of 0 and masses of 0.\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, if you set the masses to zero, then the dummy particles won't move at all throughout the simulation. Though we do apply strong harmonic positional restraints on them, I'm not sure what the effect of this is. I usually set the masses to the mass of lead -- 207 Dalton.

Also, with NonbondedForce we are setting the partial charge and LJ parameters to zero not the force constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point; I changed the mass from 207.2 to 0 at some point to see if there was any effect and never changed it back. I'll also update the comment to reflect what we're actually changing.

" ],\n",
" )\n",
" system.addForce(bond_restraint)\n",
"\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not critical, but you could probably simplify the addition of these restraints with the function apply_dat_restraints

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using OpenMM Forces directly make it a little more verbose but I find it less confusing because the two pullings are separated: automatic pulling is left up to pAPRika and the manual pulling is done directly in OpenMM. If I were to change it to pAPRika restraints do you think it would be easier for users to follow the tutorial?

Comment on lines 945 to 946
"with open(\"result.txt\", \"r\") as f:\n",
" f.write(result_string)"
Copy link
Collaborator

@jeff231li jeff231li Oct 15, 2023

Choose a reason for hiding this comment

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

I think this should be

with open("results.txt", "w") as f:
    f.write(result_string)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how I made that mistake! :)

Copy link
Collaborator

@jeff231li jeff231li left a comment

Choose a reason for hiding this comment

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

@jaketanderson Looks good. Once you make the changes go ahead and merge the PR.

@slochower
Copy link
Member

slochower commented Oct 15, 2023 via email

@jeff231li jeff231li merged commit 17ae4e3 into GilsonLabUCSD:master Nov 29, 2023
7 checks passed
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.

None yet

3 participants