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

[BUG?] Does the alanine dipeptide test system have a mixture of combining rules? #155

Closed
lohedges opened this issue Jan 30, 2024 · 5 comments · Fixed by #157
Closed

[BUG?] Does the alanine dipeptide test system have a mixture of combining rules? #155

lohedges opened this issue Jan 30, 2024 · 5 comments · Fixed by #157
Labels
bug Something isn't working

Comments

@lohedges
Copy link
Contributor

lohedges commented Jan 30, 2024

I don't know if this is an issue or not, but it confused me. If I load our canonical alanine dipeptide test system then I get no warning from Sire. However, on writing to file and reading back in I now get a warning regarding the presence of both arithmetic and geometric combining rules being present in the file. Assuming that we don't modify the existing information, why is this not the case first time around?

In [1]: import sire as sr

In [2]: mols = sr.load_test_files("ala.top", "ala.crd")

In [3]: sr.save(mols, "test.prm7")
Out[3]: ['/home/lester/Code/openbiosim/sire/test.prm7']

In [4]: sr.save(mols, "test.rst7")
Out[4]: ['/home/lester/Code/openbiosim/sire/test.rst7']

In [5]: mols = sr.load(["test.prm7", "test.rst7"])

WARNINGS encountered when parsing the topology:
The LJ parameters in this Amber Parm file use both arithmetic and geometric combining rules. Sire will use arithmetic combining rules for all LJ parameters.====


Silence these warnings by passing `show_warnings=False` when loading.
@lohedges lohedges added the bug Something isn't working label Jan 30, 2024
@lohedges
Copy link
Contributor Author

Ah, I see, load_test_files must have been silencing the warnings by default, hence why I never saw them.

@chryswoods
Copy link
Contributor

I am investigating this separately as I don't think the file does contain a mixture (but my tests last night suggested it did). There is also weirdness that writing and then re-reading the file gives the same warning, despite writing definitely using arithmetic... I think we can keep this issue open while resolving whether this is a bug, and if so, then fixing it.

@chryswoods chryswoods reopened this Jan 30, 2024
@chryswoods chryswoods changed the title [BUG] Read-write inconsistency for LJ combining rules warning [BUG?] Does the alanine dipeptide test system have a mixture of combining rules? Jan 30, 2024
@chryswoods
Copy link
Contributor

I've found and fixed the problem. The test to check if arithmetic rules weren't used was tripped accidentally for ghost atoms. This is because 0.5( sigma_a + sigma_b ) != 0 when sigma_a or sigma_b are zero, while the PRMTOP format records this as being zero. In contrast, for geometric rules, sqrt(sigma_a * sigma_b) is zero, hence why the code thought that geometric rules were used.

I've fixed it by adding a condition to check when both the PRMTOP and the combining rules say that epsilon_ij is zero, and to not then check further for exceptions or combining rules.

The impact of this bug is just the warning message. It didn't change the combining rules. It is more of an annoyance. I've nearly got feature_force_lever working so will merge it in with that.

@lohedges
Copy link
Contributor Author

Great, no problem. Glad I wasn't being stupid. I assumed that I just hadn't noticed it since the test molecules were loaded with warnings silenced.

@chryswoods
Copy link
Contributor

Here's the diff if you want to patch locally: 0feecf8?diff=unified&w=0

@chryswoods chryswoods linked a pull request Feb 8, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants