Skip to content

Add plugin for gromacs#1155

Closed
Ericwang6 wants to merge 46 commits intodeepmodeling:develfrom
Ericwang6:gmx
Closed

Add plugin for gromacs#1155
Ericwang6 wants to merge 46 commits intodeepmodeling:develfrom
Ericwang6:gmx

Conversation

@Ericwang6
Copy link
Member

Add plugin for gromacs and relevant docs / examples

Ericwang6 and others added 30 commits June 2, 2021 19:29
@Ericwang6
Copy link
Member Author

4. Examples can be smaller.

Well, do you mean reducing the number of atoms in our examples? I believe that methane in water is the smallest example to illustrate a DP/MM simulation.

@Ericwang6
Copy link
Member Author

2. The current CMake system is not used. For example, make gromacs to generate the plugin like make lammps.

Here, the plugin is designed to provide scripts for users to manually modify the source code of gromacs. And I don't think it's necessary to use the cmake system.

@njzjz
Copy link
Member

njzjz commented Sep 16, 2021

  1. For json.hpp: I think there is already one in third_party directory.

The version here is 3.9.1, which is higher than that in third_party directory. And that old version is incompatible.

Upgrading old version to new one?

@njzjz
Copy link
Member

njzjz commented Sep 16, 2021

  1. I don't think it's necessary to separate different versions. Preprocessor macros should be enough.

@AnguseZhang and I both prefer to keep version-based patches as plumed does. Because there may be too may differences between versions in GROMACS... In this way can we ensure that users of different versions of GROMACS can use our plugin.

I think deepmd_plugin.cpp and deepmd_plugin.h is not a version-based file, and all patch files can be merged into one file.

@Ericwang6
Copy link
Member Author

  1. For json.hpp: I think there is already one in third_party directory.

The version here is 3.9.1, which is higher than that in third_party directory. And that old version is incompatible.

Upgrading old version to new one?

Sure. I'm not sure whether it will cause incompatibility for deepmd-kit. But I can test on it first.

@Ericwang6
Copy link
Member Author

Ericwang6 commented Sep 16, 2021

  1. I don't think it's necessary to separate different versions. Preprocessor macros should be enough.

@AnguseZhang and I both prefer to keep version-based patches as plumed does. Because there may be too may differences between versions in GROMACS... In this way can we ensure that users of different versions of GROMACS can use our plugin.

I think deepmd_plugin.cpp and deepmd_plugin.h is not a version-based file, and all patch files can be merged into one file.

OK. Would you plz provide a reference on how to merge the patches into one file? I don't know how to do it actually...

@njzjz
Copy link
Member

njzjz commented Sep 16, 2021

git-format-patch

@Ericwang6 Ericwang6 marked this pull request as draft September 16, 2021 16:31
@Ericwang6
Copy link
Member Author

@njzjz Uploading json.hpp to 3.9.1 will cause compilation errors in deepmd-kit

@Ericwang6 Ericwang6 marked this pull request as ready for review September 16, 2021 17:22
@amcadmus amcadmus requested a review from njzjz September 17, 2021 00:40
@amcadmus
Copy link
Member

amcadmus commented Sep 17, 2021

  1. For json.hpp: I think there is already one in third_party directory.

The version here is 3.9.1, which is higher than that in third_party directory. And that old version is incompatible.

which part is incompatible? plz provide more on how you tested and the error log.

currently json.hpp is used by dp_ipi. We may want to upgrade dp_ipi accordingly.

@Ericwang6 Ericwang6 marked this pull request as draft September 17, 2021 08:16
@Ericwang6
Copy link
Member Author

Follow @njzjz 's advice, I've refactored the code in order to compile the gmx plugin along with deepmd-kit. Therefore, the number of changes and commits become too many to track for reviewers. So I'll close this pr first and open another pr for review.

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.

4 participants