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

PBRadii #1230

Merged
merged 5 commits into from Feb 16, 2022
Merged

PBRadii #1230

merged 5 commits into from Feb 16, 2022

Conversation

Valdes-Tresanco-MS
Copy link
Contributor

Sorry for the previous PR, for some reason Pycharm was always creating the new branch from v3.4.

Fix #1223

This is a contribution of @marioernestovaldes

Add mbondi_pb2, mbondi_pb3 and charmm_radii.

The implementation has a large number of comments and the definitions are explicit and redundant, which helps us to do a better follow-up. After doing more tests we will optimize the code

@swails
Copy link
Contributor

swails commented Feb 16, 2022

Thanks!

It is probably also worth adding a few tests here to support any future efforts to refactor. That can be added later, though.

@swails swails merged commit 5e48857 into ParmEd:master Feb 16, 2022
@Valdes-Tresanco-MS
Copy link
Contributor Author

Sure. We'll be working on that a bit later. Now we are preparing some examples for a possible problem that we have found in the conversion of topologies from Gromacs to Amber when cysteines are forming S-S. This is critical since the VDWAALS energies obtained using the tleap and parmed topologies differ considerably (~7000 kcal for the system in which we noticed the problem). The good news is that the delta (Complex - Receptor - Ligand) is the same for both cases.

I'll write this homework down to do later :)

@swails
Copy link
Contributor

swails commented Feb 16, 2022

Sounds like it's not creating the S-S bond

@Valdes-Tresanco-MS
Copy link
Contributor Author

Sounds like it's not creating the S-S bond

In order to be able to discuss in more detail, identify the problem and solve it, we are verifying in some systems that the problem is indeed in the CYX. As soon as we have it, we will open an issue with the details and examples. And as always, if possible a PR.

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.

Add new PBRadii sets
2 participants