Cassandra gmso #756

emarinri · 2023-08-11T19:28:04Z

The goal of this PR is to finish up the MCF writer of GMSO originally started by #560

* Add fixed angle support * Fix charge sign error (gmso-wide) * Update ring identification to handle fused systems (mbuild mosdef-hub#744) * Increase floating point accuracy * Increase element type length and atom type length * Correctly write atom indices for bonds/angles/dihedrals/impropers

for more information, see https://pre-commit.ci

* Modify dihedral constant indexing from k0 to k3 to k1 to k4 in the OPLSTorsionPotential template.

* Updated from old Mie Xenon test to OPLS ethane. * Remove assertions related to n and m parameters of the Mie potential. * Fix floating point format issues in MCF writer.

for more information, see https://pre-commit.ci

gmso/tests/test_mcf.py

codecov · 2023-08-11T19:55:51Z

Codecov Report

Patch coverage: 77.89% and project coverage change: +0.83% 🎉

Comparison is base (ff94811) 91.99% compared to head (f38cfb6) 92.82%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #756      +/-   ##
==========================================
+ Coverage   91.99%   92.82%   +0.83%     
==========================================
  Files          66       66              
  Lines        6806     6845      +39     
==========================================
+ Hits         6261     6354      +93     
+ Misses        545      491      -54

Files Changed	Coverage Δ
gmso/utils/io.py	`94.36% <50.00%> (-2.65%)`	⬇️
gmso/formats/mcf.py	`82.86% <79.12%> (+28.70%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This test is not complete yet. The 1/2 factor in the angles and OPLS dihedral is not accounted for.

Co-authored-by: Co Quach <43968221+daico007@users.noreply.github.com>

CalCraven

Looking good, thanks for getting on this Eliseo. I think a few changes to use more of the GMSO functionality that's there will help speed this up a lot. Also need a few tests for things we expect to fail.

gmso/formats/mcf.py

gmso/tests/test_mcf.py

CalCraven · 2023-08-21T15:42:35Z

In terms of just getting a single one of each molecule, here is something you can use. It could be a method just in the mcf writer, or something that's a method for the Topology class.

subtopsList = []
for molecule in top.unique_site_labels(name_only=True):
        subtopsList.append(top.create_subtop("molecule", (molecule, 1)))

CalCraven · 2023-09-05T18:51:30Z

Is there any way to add a test that tests that the mcf files can actually be read into Cassandra. Something like the tests for the hoomd simulation in mBuild (https://github.com/mosdef-hub/mbuild/blob/1d744186e0c92c6fb5e704aef2c76eaa22cee802/mbuild/tests/test_hoomd.py#L197).

We probably also need to test MCF for an array of forcefields or molecules types.

daico007 · 2023-09-07T20:51:39Z

I think since canssandra is conda installable, we should be able to run a short sim (through mosdef_cassandra probably) to confirm the file written out is correctly formatted.

daico007 · 2023-09-11T19:56:46Z

I am pulling this PR for local testing. At first test, things seem to write out as expected (through inspection of the mcf file), however, there might some performance, as in it takes 8s to write out a system of one ethanol. I am doing some profiling right now and going through the code to see if there is something can be sped up. I might try to add some test, i.e., running a cassandra simulation, but since I can't install cassandra locally (there isn't a ARM distribution), it might or might not work.

daico007 · 2023-09-11T20:28:43Z

Hi @emarinri, can you check and confirm the conversion of k when writing out dihedrals and improper. Currently, our json has the OPLS dihedral with the form of:

{
  "name": "OPLSTorsionPotential",
  "expression": "0.5 * k1 * (1 + cos(phi)) + 0.5 * k2 * (1 - cos(2*phi)) + 0.5 * k3 * (1 + cos(3*phi)) + 0.5 * k4 * (1 - cos(4*phi))",
  "independent_variables": "phi",
  "expected_parameters_dimensions": {
    "k1": "energy",
    "k2": "energy",
    "k3": "energy",
    "k4": "energy"
  }
}

while the Cassandra forcefield is using 𝐸𝜙=𝑎0+𝑎1(1+cos(𝜙))+𝑎2(1−cos(2𝜙))+𝑎3(1+cos(3𝜙))
(https://cassandra-mc.readthedocs.io/en/latest/guides/forcefield.html), so without the 1/2.

However, the mcf.py currently scale down the number by another 1/2 (line 490-515), shouldn't these k be multiplied by 2 to match with the cassandra form?

gmso/tests/test_mcf.py

for more information, see https://pre-commit.ci

gmso/utils/io.py

…o cassandra-gmso

gmso/tests/test_mcf.py

CalCraven

Adding the tests to read it into cassandra itself look pretty good. I think there's probably more edge cases to add, but we may have to add them as they come. Some of these test are going to be covered more in the mosdef_cassandra package as well.

gmso/tests/test_mcf.py

daico007

I think this is a good wrapping up point for this PR unless you want to add in more stuff @emarinri.

gmso/tests/test_mcf.py

daico007 · 2023-09-22T02:26:42Z

gmso/tests/test_mcf.py

+        # TODO: not sure why the cassandra MCF writer of mBuild
+        # outputs a different intramolecular exclusions relative
+        # to the GMSO writer. This is a temporary fix to make the
+        # test pass. We should investigate this further.
+        # Also, try to use the function top.set_lj_scale() and see how
+        # to update subtopologies.


Is this something we need to look into now?

I do not think this is something we have to worry about for now.

emarinri · 2023-09-22T09:43:32Z

@daico007 I agree this is a point where we can wrap this up for now. As you mentioned, there are a few more things to be tested for should be OK for the short term.

rsdefever and others added 7 commits August 11, 2023 11:28

Add fixed length bonds and angles to potential templates library.

665feb6

[pre-commit.ci] auto fixes from pre-commit.com hooks

a6ce48d

for more information, see https://pre-commit.ci

Add dimensions to Fixed Bond and Angle templates

a97826c

Update keys in OPLS dihedral parameters dict

c9bd1d1

* Modify dihedral constant indexing from k0 to k3 to k1 to k4 in the OPLSTorsionPotential template.

Fix typed OPLS ethane test for MCF format

014b7ae

* Updated from old Mie Xenon test to OPLS ethane. * Remove assertions related to n and m parameters of the Mie potential. * Fix floating point format issues in MCF writer.

[pre-commit.ci] auto fixes from pre-commit.com hooks

f41d5a9

for more information, see https://pre-commit.ci

github-advanced-security bot found potential problems Aug 11, 2023

View reviewed changes

gmso/tests/test_mcf.py Fixed Show fixed Hide fixed

gmso/tests/test_mcf.py Fixed Show fixed Hide fixed

gmso/tests/test_mcf.py Fixed Show fixed Hide fixed

emarinri and others added 4 commits August 13, 2023 14:06

Add typed ethane test.

f8e044c

This test is not complete yet. The 1/2 factor in the angles and OPLS dihedral is not accounted for.

Fix code style

059ee03

Add fixture to parse mcf into sections

48fd600

Support Pydantic v1 and v2 (mosdef-hub#752)

aed4c44

Co-authored-by: Co Quach <43968221+daico007@users.noreply.github.com>

CalCraven requested changes Aug 14, 2023

View reviewed changes

gmso/formats/mcf.py Outdated Show resolved Hide resolved

gmso/formats/mcf.py Outdated Show resolved Hide resolved

gmso/tests/test_mcf.py Show resolved Hide resolved

emarinri added 7 commits August 15, 2023 12:23

Update MCF with GMSO builtin top site sorting

76415a8

Use PotentialFilters to check for unique atomtypes

f093262

Add test to check charge neutrality

44a0928

Add test for incompatible expressions

b9efc1b

Test full MCF for Mie-Xe and LJ-Ar

f9f2759

Check charge neutrality in each molecule test

5442297

Add exception for not neutral system in MCF writer

86902e6

github-advanced-security bot found potential problems Aug 16, 2023

View reviewed changes

gmso/tests/test_mcf.py Fixed Show fixed Hide fixed

Move parsing and neutrality check as utils

00cc2bc

Merge branch 'main' into cassandra-gmso

73d24de

Merge branch 'main' into cassandra-gmso

6094aba

github-advanced-security bot found potential problems Sep 14, 2023

View reviewed changes

gmso/tests/test_mcf.py Dismissed Show dismissed Hide dismissed

CalCraven and others added 2 commits September 18, 2023 21:50

Add cassandra test for gmso

ae69a8c

[pre-commit.ci] auto fixes from pre-commit.com hooks

750d755

for more information, see https://pre-commit.ci

github-advanced-security bot found potential problems Sep 18, 2023

View reviewed changes

gmso/utils/io.py Dismissed Show dismissed Hide dismissed

emarinri and others added 14 commits September 19, 2023 14:00

Add test for two atom fragment

a7df9bc

Test for molecule with one ring

685f8b8

Change nitrogen test to ethane ua

1252d2b

Add ethane rigid reference xml

ec3a5dd

Use Fourier dihedrals as MCF standard dihedrals.

a30d7bd

Fix dihedral type output fourier to opls

64b5b8f

Match MCF GMSO Angle header to mb Angle header

ad68575

Add a test comparing gmso and mbuild mcf writers

0759365

Use the Fourier converter for ethylene dihedrals

cf4e0c0

Write atom type masses instead of atom masses to account for UA beads

f462d5b

Output correct case for dihedral styles in MCFs

2a40bbf

Merge branch 'main' into cassandra-gmso

c12a120

Merge branch 'cassandra-gmso' of https://github.com/emarinri/gmso int…

6ff41e2

…o cassandra-gmso

Run an energy calculation to compare GMSO and mBuild MCF writers

c1b9c19

github-advanced-security bot found potential problems Sep 20, 2023

View reviewed changes

gmso/tests/test_mcf.py Fixed Show resolved Hide resolved

gmso/tests/test_mcf.py Fixed Show fixed Hide fixed

CalCraven approved these changes Sep 21, 2023

View reviewed changes

gmso/tests/test_mcf.py Fixed Show resolved Hide resolved

Merge branch 'main' into cassandra-gmso

dcdc342

daico007 approved these changes Sep 21, 2023

View reviewed changes

gmso/tests/test_mcf.py Show resolved Hide resolved

daico007 reviewed Sep 21, 2023

View reviewed changes

gmso/tests/test_mcf.py Show resolved Hide resolved

daico007 added 2 commits September 21, 2023 16:54

Update gmso/tests/test_mcf.py

3c12947

Update gmso/tests/test_mcf.py

a1f4853

daico007 reviewed Sep 22, 2023

View reviewed changes

Merge branch 'main' into cassandra-gmso

f38cfb6

daico007 merged commit 7537899 into mosdef-hub:main Sep 22, 2023
15 of 16 checks passed

daico007 mentioned this pull request Sep 22, 2023

Make MCF writer feature complete #560

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cassandra gmso #756

Cassandra gmso #756

emarinri commented Aug 11, 2023

codecov bot commented Aug 11, 2023 •

edited

CalCraven left a comment

CalCraven commented Aug 21, 2023

CalCraven commented Sep 5, 2023

daico007 commented Sep 7, 2023

daico007 commented Sep 11, 2023

daico007 commented Sep 11, 2023

CalCraven left a comment

daico007 left a comment

daico007 Sep 22, 2023

emarinri Sep 22, 2023

emarinri commented Sep 22, 2023

Cassandra gmso #756

Cassandra gmso #756

Conversation

emarinri commented Aug 11, 2023

codecov bot commented Aug 11, 2023 • edited

Codecov Report

CalCraven left a comment

Choose a reason for hiding this comment

CalCraven commented Aug 21, 2023

CalCraven commented Sep 5, 2023

daico007 commented Sep 7, 2023

daico007 commented Sep 11, 2023

daico007 commented Sep 11, 2023

CalCraven left a comment

Choose a reason for hiding this comment

daico007 left a comment

Choose a reason for hiding this comment

daico007 Sep 22, 2023

Choose a reason for hiding this comment

emarinri Sep 22, 2023

Choose a reason for hiding this comment

emarinri commented Sep 22, 2023

codecov bot commented Aug 11, 2023 •

edited