From e100a42087f0bf75a815fbe95cd837ba8ac2d22b Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 16 Dec 2023 17:07:37 -0500 Subject: [PATCH 01/15] (re)throw EOF exception when next_dvector() has not yet read any items --- src/potential_file_reader.cpp | 2 ++ src/text_file_reader.cpp | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/potential_file_reader.cpp b/src/potential_file_reader.cpp index 2c0b9a6a55..613225a797 100644 --- a/src/potential_file_reader.cpp +++ b/src/potential_file_reader.cpp @@ -144,6 +144,8 @@ void PotentialFileReader::next_dvector(double *list, int n) { try { return reader->next_dvector(list, n); + } catch (EOFException &) { + throw EOFException("EOF reached"); } catch (FileReaderException &e) { error->one(FLERR, e.what()); } diff --git a/src/text_file_reader.cpp b/src/text_file_reader.cpp index 46a5fd33a9..0b8d717687 100644 --- a/src/text_file_reader.cpp +++ b/src/text_file_reader.cpp @@ -189,8 +189,9 @@ void TextFileReader::next_dvector(double *list, int n) char *ptr = next_line(); if (ptr == nullptr) { - // EOF - if (i < n) { + if (i == 0) { // EOF without any records + throw EOFException("EOF reached"); + } else if (i < n) { // EOF with incomplete data throw FileReaderException( fmt::format("Incorrect format in {} file! {}/{} values", filetype, i, n)); } From 96ef731f069c2d65121c8a0e7041f94e4e235df9 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 16 Dec 2023 17:07:53 -0500 Subject: [PATCH 02/15] remove unused items --- src/MOLECULE/fix_cmap.cpp | 11 ----------- src/MOLECULE/fix_cmap.h | 3 +-- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/src/MOLECULE/fix_cmap.cpp b/src/MOLECULE/fix_cmap.cpp index f74c05ef06..edd52ee47b 100644 --- a/src/MOLECULE/fix_cmap.cpp +++ b/src/MOLECULE/fix_cmap.cpp @@ -87,9 +87,6 @@ FixCMAP::FixCMAP(LAMMPS *lmp, int narg, char **arg) : respa_level_support = 1; ilevel_respa = 0; - MPI_Comm_rank(world,&me); - MPI_Comm_size(world,&nprocs); - // allocate memory for CMAP data memory->create(g_axis,CMAPDIM,"cmap:g_axis"); @@ -184,10 +181,6 @@ void FixCMAP::init() for (i = 0; i < 6; i++) set_map_derivatives(cmapgrid[i],d1cmapgrid[i],d2cmapgrid[i],d12cmapgrid[i]); - // define newton_bond here in case restart file was read (not data file) - - newton_bond = force->newton_bond; - if (utils::strmatch(update->integrate_style,"^respa")) { ilevel_respa = (dynamic_cast(update->integrate))->nlevels-1; if (respa_level >= 0) ilevel_respa = MIN(respa_level,ilevel_respa); @@ -934,10 +927,6 @@ void FixCMAP::read_data_header(char *line) } catch (std::exception &e) { error->all(FLERR,"Invalid read data header line for fix cmap: {}", e.what()); } - - // not set in constructor because this fix could be defined before newton command - - newton_bond = force->newton_bond; } /* ---------------------------------------------------------------------- diff --git a/src/MOLECULE/fix_cmap.h b/src/MOLECULE/fix_cmap.h index fce76aa540..1c6aba95e0 100644 --- a/src/MOLECULE/fix_cmap.h +++ b/src/MOLECULE/fix_cmap.h @@ -65,8 +65,7 @@ class FixCMAP : public Fix { double memory_usage() override; private: - int nprocs, me; - int newton_bond, eflag_caller; + int eflag_caller; int ctype, ilevel_respa; int ncrosstermtypes, crossterm_per_atom, maxcrossterm; int ncrosstermlist; From fb6a5843b9d5457bded5aa323233f40ff728faaa Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 16 Dec 2023 17:14:47 -0500 Subject: [PATCH 03/15] handle comments in CMAP coeff assignments --- src/MOLECULE/fix_cmap.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/MOLECULE/fix_cmap.cpp b/src/MOLECULE/fix_cmap.cpp index edd52ee47b..cfc71f96fc 100644 --- a/src/MOLECULE/fix_cmap.cpp +++ b/src/MOLECULE/fix_cmap.cpp @@ -946,10 +946,10 @@ void FixCMAP::read_data_section(char * /*keyword*/, int /*n*/, char *buf, // loop over lines of CMAP crossterms // tokenize the line into values - // add crossterm to one of my atoms, depending on newton_bond + // add crossterm to one of my atoms for (const auto &line : lines) { - ValueTokenizer values(line); + ValueTokenizer values(utils::trim_comment(line)); try { values.skip(); itype = values.next_int(); From 4bf1b1d9c0d058ceaba03f1976709b3868684fa1 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 16 Dec 2023 17:15:31 -0500 Subject: [PATCH 04/15] some refactoring and modernization --- src/MOLECULE/fix_cmap.cpp | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/MOLECULE/fix_cmap.cpp b/src/MOLECULE/fix_cmap.cpp index cfc71f96fc..1f137cb833 100644 --- a/src/MOLECULE/fix_cmap.cpp +++ b/src/MOLECULE/fix_cmap.cpp @@ -49,15 +49,14 @@ using namespace LAMMPS_NS; using namespace FixConst; using namespace MathConst; -#define MAXLINE 256 -#define LISTDELTA 10000 -#define LB_FACTOR 1.5 +static constexpr int LISTDELTA = 10000; +static constexpr double LB_FACTOR = 1.5; -#define CMAPMAX 6 // max # of CMAP terms stored by one atom -#define CMAPDIM 24 // grid map dimension is 24 x 24 -#define CMAPXMIN -360.0 -#define CMAPXMIN2 -180.0 -#define CMAPDX 15.0 // 360/CMAPDIM +static constexpr int CMAPMAX = 6; // max # of CMAP terms stored by one atom +static constexpr int CMAPDIM = 24; // grid map dimension is 24 x 24 +static constexpr double CMAPXMIN = -360.0; +static constexpr double CMAPXMIN2 = -180.0; +static constexpr double CMAPDX = 15.0; // 360/CMAPDIM /* ---------------------------------------------------------------------- */ @@ -90,10 +89,10 @@ FixCMAP::FixCMAP(LAMMPS *lmp, int narg, char **arg) : // allocate memory for CMAP data memory->create(g_axis,CMAPDIM,"cmap:g_axis"); - memory->create(cmapgrid,6,CMAPDIM,CMAPDIM,"cmap:grid"); - memory->create(d1cmapgrid,6,CMAPDIM,CMAPDIM,"cmap:d1grid"); - memory->create(d2cmapgrid,6,CMAPDIM,CMAPDIM,"cmap:d2grid"); - memory->create(d12cmapgrid,6,CMAPDIM,CMAPDIM,"cmap:d12grid"); + memory->create(cmapgrid,CMAPMAX,CMAPDIM,CMAPDIM,"cmap:grid"); + memory->create(d1cmapgrid,CMAPMAX,CMAPDIM,CMAPDIM,"cmap:d1grid"); + memory->create(d2cmapgrid,CMAPMAX,CMAPDIM,CMAPDIM,"cmap:d2grid"); + memory->create(d12cmapgrid,CMAPMAX,CMAPDIM,CMAPDIM,"cmap:d12grid"); // read and setup CMAP data @@ -231,6 +230,8 @@ void FixCMAP::min_setup(int vflag) void FixCMAP::pre_neighbor() { int i,m,atom1,atom2,atom3,atom4,atom5; + const int me = comm->me; + const int nprocs = comm->nprocs; // guesstimate initial length of local crossterm list // if ncmap was not set (due to read_restart, no read_data), From 1ab406ee1acbdbb47208b219ba3b5f33f8ecb627 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 16 Dec 2023 17:16:06 -0500 Subject: [PATCH 05/15] read CMAP data blocks one at a time and catch EOF exception to stop reading --- src/MOLECULE/fix_cmap.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/MOLECULE/fix_cmap.cpp b/src/MOLECULE/fix_cmap.cpp index 1f137cb833..01b62d2a28 100644 --- a/src/MOLECULE/fix_cmap.cpp +++ b/src/MOLECULE/fix_cmap.cpp @@ -39,6 +39,7 @@ #include "memory.h" #include "potential_file_reader.h" #include "respa.h" +#include "text_file_reader.h" #include "update.h" #include @@ -631,15 +632,22 @@ void FixCMAP::read_grid_map(char *cmapfile) { if (comm->me == 0) { try { - memset(&cmapgrid[0][0][0], 0, 6*CMAPDIM*CMAPDIM*sizeof(double)); + ncrosstermtypes = 0; + memset(&cmapgrid[0][0][0], 0, CMAPMAX*CMAPDIM*CMAPDIM*sizeof(double)); + utils::logmesg(lmp, "Reading CMAP parameters from: {}\n", cmapfile); PotentialFileReader reader(lmp, cmapfile, "cmap grid"); - // there are six maps in this order. + // there may be up to six maps. + // the charmm36.cmap file has in this order. // alanine, alanine-proline, proline, proline-proline, glycine, glycine-proline. - // read as one big blob of numbers while ignoring comments - - reader.next_dvector(&cmapgrid[0][0][0],6*CMAPDIM*CMAPDIM); + // custom CMAP files created by charmm-gui may have fewer entries + // read one map at a time as a blob of numbers while ignoring comments + // and stop reading when whe have reached EOF. + for (ncrosstermtypes = 0; ncrosstermtypes < CMAPMAX; ++ncrosstermtypes) + reader.next_dvector(&cmapgrid[ncrosstermtypes][0][0],CMAPDIM*CMAPDIM); + } catch (EOFException &) { + utils::logmesg(lmp, " Read in CMAP data for {} crossterm types\n", ncrosstermtypes); } catch (std::exception &e) { error->one(FLERR,"Error reading CMAP potential file: {}", e.what()); } From 695a81ef70b3a97b094c4e42ef82c8b9e603a649 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 16 Dec 2023 21:50:38 -0500 Subject: [PATCH 06/15] avoid uninitialized data access --- src/MOLECULE/fix_cmap.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/MOLECULE/fix_cmap.cpp b/src/MOLECULE/fix_cmap.cpp index 01b62d2a28..cb4cb8cadc 100644 --- a/src/MOLECULE/fix_cmap.cpp +++ b/src/MOLECULE/fix_cmap.cpp @@ -86,6 +86,7 @@ FixCMAP::FixCMAP(LAMMPS *lmp, int narg, char **arg) : wd_section = 1; respa_level_support = 1; ilevel_respa = 0; + eflag_caller = 1; // allocate memory for CMAP data From 58ed034d7a92718f075e237076b88ff669cf022b Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Sun, 17 Dec 2023 15:42:10 -0600 Subject: [PATCH 07/15] Updated Developer unittest for debugging failed unit tests individually --- doc/src/Developer_unittest.rst | 66 ++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/doc/src/Developer_unittest.rst b/doc/src/Developer_unittest.rst index e311fcdfc5..b517ca1802 100644 --- a/doc/src/Developer_unittest.rst +++ b/doc/src/Developer_unittest.rst @@ -526,3 +526,69 @@ The ``unittest/tools`` folder contains tests for programs in the shell, which are implemented as a python scripts using the ``unittest`` Python module and launching the tool commands through the ``subprocess`` Python module. + + +Troubleshooting failed unit tests +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The unit tests of a newly added features (e.g. pair, fix or compute styles) +are not automatically run when your pull request (PR) is submitted via GitHub. +To trigger the unit test(s), you need to add appropriate labels the PR, e.g., +``gpu_unit_tests`` to enable the unit tests for the GPU package. After the test +has started, you had better remove the label since every PR activity will +retrigger the test. + +For a failed build on GitHub, click on Details to go to the LAMMPS Jenkins CI web page. +Click on the "Exit" symbol near the "Logout" button on the top right of that page +to go to the classic view. In the classic view, there is a list of the individual runs +that make up this test run. You can click on any of those. Then on "Test Result" +to display the list of tests. Click on the Status column to sort the tests +based on their Failed or Passed status. Then click on the failed test to expand +its output. + +For example, the following output snippet shows the failed unit test + +.. code-block:: bash + + [ RUN ] PairStyle.gpu + /home/builder/workspace/dev/pull_requests/ubuntu_gpu/unit_tests/cmake_gpu_opencl_mixed_smallbig_clang_static/unittest/force-styles/test_main.cpp:63: Failure + Expected: (err) <= (epsilon) + Actual: 0.00018957912910606503 vs 0.0001 + Google Test trace: + /home/builder/workspace/dev/pull_requests/ubuntu_gpu/unit_tests/cmake_gpu_opencl_mixed_smallbig_clang_static/unittest/force-styles/test_main.cpp:56: EXPECT_FORCES: init_forces (newton off) + /home/builder/workspace/dev/pull_requests/ubuntu_gpu/unit_tests/cmake_gpu_opencl_mixed_smallbig_clang_static/unittest/force-styles/test_main.cpp:64: Failure + Expected: (err) <= (epsilon) + Actual: 0.00022892713393549854 vs 0.0001 + + +Note that the force style engine runs the same system on a rather +off-equilibrium system with few atoms for just a few steps, writes data and restart, +uses ``clean`` to reset, and then run with different settings and integrators. +Beyond potential issues/bugs in the source code, the mismatch between the expected +and actual values could be that force arrays are not properly cleared between +multiple run commands and/or when starting a run from restarts. + +As a rule of thumb, the test epsilon can often be in the range 5e-14 to 1e-13. +For "noisy" force kernels, e.g. those involving `exp()`, `log()` or `sin()` +operations and subject to compiler optimization, epsilon can be further relaxed, +sometimes epsilon can be relaxed to 1.e-12. If lookup tables are added, +we may need to go to 1.e-10 or even higher. + +To rerun the failed unit test individually, change to the ``build`` directory +and run the test with verbose output. For example, + +.. code-block:: bash + + env TEST_ARGS=-v ctest -R ^MolPairStyle:lj_cut_coul_long -V + +It is recommended to configure the build with ``-D BUILD_SHARED_LIBS=on`` to +shorten the build time during recompilation. Installing `ccache` in +your development environment helps speed up recompilation by +caching previous compilations and detecting when the same compilation +is being done again. + + + + + + From 188e1090e9e37272144890a33479670cc592dde9 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sun, 17 Dec 2023 20:01:47 -0500 Subject: [PATCH 08/15] add some corrections and clarifications --- doc/src/Build_development.rst | 41 +++++++++------- doc/src/Developer_unittest.rst | 90 ++++++++++++++++++++++------------ 2 files changed, 80 insertions(+), 51 deletions(-) diff --git a/doc/src/Build_development.rst b/doc/src/Build_development.rst index d30a433d62..c674b2c258 100644 --- a/doc/src/Build_development.rst +++ b/doc/src/Build_development.rst @@ -255,16 +255,18 @@ A test run is then a a collection multiple individual test runs each with many comparisons to reference results based on template input files, individual command settings, relative error margins, and reference data stored in a YAML format file with ``.yaml`` -suffix. Currently the programs ``test_pair_style``, ``test_bond_style``, and -``test_angle_style`` are implemented. They will compare forces, energies and -(global) stress for all atoms after a ``run 0`` calculation and after a -few steps of MD with :doc:`fix nve `, each in multiple variants -with different settings and also for multiple accelerated styles. If a -prerequisite style or package is missing, the individual tests are -skipped. All tests will be executed on a single MPI process, so using -the CMake option ``-D BUILD_MPI=off`` can significantly speed up testing, -since this will skip the MPI initialization for each test run. -Below is an example command and output: +suffix. Currently the programs ``test_pair_style``, ``test_bond_style``, +``test_angle_style``, ``test_dihedral_style``, and +``test_improper_style`` are implemented. They will compare forces, +energies and (global) stress for all atoms after a ``run 0`` calculation +and after a few steps of MD with :doc:`fix nve `, each in +multiple variants with different settings and also for multiple +accelerated styles. If a prerequisite style or package is missing, the +individual tests are skipped. All force style tests will be executed on +a single MPI process, so using the CMake option ``-D BUILD_MPI=off`` can +significantly speed up testing, since this will skip the MPI +initialization for each test run. Below is an example command and +output: .. code-block:: console @@ -416,15 +418,16 @@ When compiling LAMMPS with enabled tests, most test executables will need to be linked against the LAMMPS library. Since this can be a very large library with many C++ objects when many packages are enabled, link times can become very long on machines that use the GNU BFD linker (e.g. -Linux systems). Alternatives like the ``lld`` linker of the LLVM project -or the ``gold`` linker available with GNU binutils can speed up this step -substantially. CMake will by default test if any of the two can be -enabled and use it when ``ENABLE_TESTING`` is active. It can also be -selected manually through the ``CMAKE_CUSTOM_LINKER`` CMake variable. -Allowed values are ``lld``, ``gold``, ``bfd``, or ``default``. The -``default`` option will use the system default linker otherwise, the -linker is chosen explicitly. This option is only available for the -GNU or Clang C++ compiler. +Linux systems). Alternatives like the ``mold`` linker, the ``lld`` +linker of the LLVM project, or the ``gold`` linker available with GNU +binutils can speed up this step substantially (in this order). CMake +will by default test if any of the three can be enabled and use it when +``ENABLE_TESTING`` is active. It can also be selected manually through +the ``CMAKE_CUSTOM_LINKER`` CMake variable. Allowed values are +``mold``, ``lld``, ``gold``, ``bfd``, or ``default``. The ``default`` +option will use the system default linker otherwise, the linker is +chosen explicitly. This option is only available for the GNU or Clang +C++ compilers. Tests for other components and utility functions ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/doc/src/Developer_unittest.rst b/doc/src/Developer_unittest.rst index b517ca1802..c0cdee6c09 100644 --- a/doc/src/Developer_unittest.rst +++ b/doc/src/Developer_unittest.rst @@ -121,7 +121,7 @@ will be suppressed and only a summary printed, but adding the '-V' option will then produce output from the tests above like the following: -.. code-block:: +.. code-block:: console [...] 1: [ RUN ] Tokenizer.empty_string @@ -531,24 +531,33 @@ Python module. Troubleshooting failed unit tests ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -The unit tests of a newly added features (e.g. pair, fix or compute styles) -are not automatically run when your pull request (PR) is submitted via GitHub. -To trigger the unit test(s), you need to add appropriate labels the PR, e.g., -``gpu_unit_tests`` to enable the unit tests for the GPU package. After the test -has started, you had better remove the label since every PR activity will -retrigger the test. +The are by default no unit tests for newly added features (e.g. pair, fix, +or compute styles) unless your pull request also includes tests for the +added features. If you are modifying some features, you may see failures +for existing tests, if your modifications have some unexpected side effects +or your changes render the existing text invalid. If you are adding an +accelerated version of an existing style, then only tests for INTEL, +KOKKOS (with OpenMP only), OPENMP, and OPT will be run automatically. +Tests for the GPU package are time consuming and thus are only run +*after* a merge, or when a special label, ``gpu_unit_tests`` is added +to the pull request. After the test has started, it is often best to +remove the label since every PR activity will re-trigger the test (that +is a limitation of triggering a test with a label). Support for unit +tests with using KOKKOS with GPU acceleration is currently not supported. -For a failed build on GitHub, click on Details to go to the LAMMPS Jenkins CI web page. -Click on the "Exit" symbol near the "Logout" button on the top right of that page -to go to the classic view. In the classic view, there is a list of the individual runs -that make up this test run. You can click on any of those. Then on "Test Result" -to display the list of tests. Click on the Status column to sort the tests -based on their Failed or Passed status. Then click on the failed test to expand -its output. +When you see a failed build on GitHub, click on ``Details`` to be taken +to the corresponding LAMMPS Jenkins CI web page. Click on the "Exit" +symbol near the ``Logout`` button on the top right of that page to go to +the "classic view". In the classic view, there is a list of the +individual runs that make up this test run (they are shown but cannot be +inspected in the default view). You can click on any of those. +Clicking on ``Test Result`` will display the list of failed tests. Click +on the "Status" column to sort the tests based on their Failed or Passed +status. Then click on the failed test to expand its output. For example, the following output snippet shows the failed unit test -.. code-block:: bash +.. code-block:: console [ RUN ] PairStyle.gpu /home/builder/workspace/dev/pull_requests/ubuntu_gpu/unit_tests/cmake_gpu_opencl_mixed_smallbig_clang_static/unittest/force-styles/test_main.cpp:63: Failure @@ -560,19 +569,35 @@ For example, the following output snippet shows the failed unit test Expected: (err) <= (epsilon) Actual: 0.00022892713393549854 vs 0.0001 +Note that the force style engine runs one of a small number of systems +in a rather off-equilibrium configuration with a few atoms for a few +steps, writes data and restart files, uses :doc:`the clear command +` to reset LAMMPS, and then runs from those files with different +settings (e.g. newton on/off) and integrators (e.g. verlet vs. respa). +Beyond potential issues/bugs in the source code, the mismatch between +the expected and actual values could be that force arrays are not +properly cleared between multiple run commands or that class members are +not correctly initialized or written to or read from a data or restart +file. -Note that the force style engine runs the same system on a rather -off-equilibrium system with few atoms for just a few steps, writes data and restart, -uses ``clean`` to reset, and then run with different settings and integrators. -Beyond potential issues/bugs in the source code, the mismatch between the expected -and actual values could be that force arrays are not properly cleared between -multiple run commands and/or when starting a run from restarts. - -As a rule of thumb, the test epsilon can often be in the range 5e-14 to 1e-13. -For "noisy" force kernels, e.g. those involving `exp()`, `log()` or `sin()` -operations and subject to compiler optimization, epsilon can be further relaxed, -sometimes epsilon can be relaxed to 1.e-12. If lookup tables are added, -we may need to go to 1.e-10 or even higher. +While the epsilon (relative precision) for a single, `IEEE 754 compliant +`_, double precision floating +point operation is at about 2.2e-16, the achievable precision for the +tests is lower due to most numbers being sums over intermediate results +and the non-associativity of floating point math leading to larger +errors. In some cases specific properties of the tested style. As a +rule of thumb, the test epsilon can often be in the range 5.0e-14 to +1.0e-13. But for "noisy" force kernels, e.g. those a larger amount of +arithmetic operations involving `exp()`, `log()` or `sin()` functions, +and also due to the effect of compiler optimization or differences +between compilers or platforms, epsilon may need to be further relaxed, +sometimes epsilon can be relaxed to 1.0e-12. If interpolation or lookup +tables are used, epsilon may need to be set to 1.0e-10 or even higher. +For tests of accelerated styles, the per-test epsilon is multiplied +by empirical factors that take into account the differences in the order +of floating point operations or that some or most intermediate operations +may be done using approximations or with single precision floating point +math. To rerun the failed unit test individually, change to the ``build`` directory and run the test with verbose output. For example, @@ -581,11 +606,12 @@ and run the test with verbose output. For example, env TEST_ARGS=-v ctest -R ^MolPairStyle:lj_cut_coul_long -V -It is recommended to configure the build with ``-D BUILD_SHARED_LIBS=on`` to -shorten the build time during recompilation. Installing `ccache` in -your development environment helps speed up recompilation by -caching previous compilations and detecting when the same compilation -is being done again. +It is recommended to configure the build with ``-D +BUILD_SHARED_LIBS=on`` and use a custom linker to shorten the build time +during recompilation. Installing `ccache` in your development +environment helps speed up recompilation by caching previous +compilations and detecting when the same compilation is being done +again. Please see :doc:`Build_development` for further details. From ba4ac991b6b62ef1feacb016554915145c4696c5 Mon Sep 17 00:00:00 2001 From: oywg11 Date: Mon, 18 Dec 2023 16:06:36 +0800 Subject: [PATCH 09/15] small fix for ilp/tmd and KC/full --- src/INTERLAYER/pair_ilp_tmd.cpp | 2 +- src/INTERLAYER/pair_kolmogorov_crespi_full.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/INTERLAYER/pair_ilp_tmd.cpp b/src/INTERLAYER/pair_ilp_tmd.cpp index 8b08de39c0..73f89803c2 100644 --- a/src/INTERLAYER/pair_ilp_tmd.cpp +++ b/src/INTERLAYER/pair_ilp_tmd.cpp @@ -210,7 +210,7 @@ void PairILPTMD::calc_FRep(int eflag, int /* vflag */) delki[1] = x[k][1] - x[i][1]; delki[2] = x[k][2] - x[i][2]; if (evflag) - ev_tally_xyz(k, j, nlocal, newton_pair, 0.0, 0.0, fk[0], fk[1], fk[2], delki[0], + ev_tally_xyz(k, i, nlocal, newton_pair, 0.0, 0.0, fk[0], fk[1], fk[2], delki[0], delki[1], delki[2]); } diff --git a/src/INTERLAYER/pair_kolmogorov_crespi_full.cpp b/src/INTERLAYER/pair_kolmogorov_crespi_full.cpp index b497ae3568..ad42ba1922 100644 --- a/src/INTERLAYER/pair_kolmogorov_crespi_full.cpp +++ b/src/INTERLAYER/pair_kolmogorov_crespi_full.cpp @@ -590,7 +590,7 @@ void PairKolmogorovCrespiFull::calc_FRep(int eflag, int /* vflag */) delki[1] = x[k][1] - x[i][1]; delki[2] = x[k][2] - x[i][2]; if (evflag) - ev_tally_xyz(k, j, nlocal, newton_pair, 0.0, 0.0, fk[0], fk[1], fk[2], delki[0], + ev_tally_xyz(k, i, nlocal, newton_pair, 0.0, 0.0, fk[0], fk[1], fk[2], delki[0], delki[1], delki[2]); } From 8dcf980d0abc2418d8f21f1ff6f6ac7451237e6d Mon Sep 17 00:00:00 2001 From: oywg11 Date: Mon, 18 Dec 2023 16:55:10 +0800 Subject: [PATCH 10/15] update doc of ilp/tmd --- doc/src/pair_ilp_tmd.rst | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/doc/src/pair_ilp_tmd.rst b/doc/src/pair_ilp_tmd.rst index 70a4768389..dcacc18ed6 100644 --- a/doc/src/pair_ilp_tmd.rst +++ b/doc/src/pair_ilp_tmd.rst @@ -22,12 +22,12 @@ Examples .. code-block:: LAMMPS pair_style hybrid/overlay ilp/tmd 16.0 1 - pair_coeff * * ilp/tmd MoS2.ILP Mo S S + pair_coeff * * ilp/tmd TMD.ILP Mo S S pair_style hybrid/overlay sw/mod sw/mod ilp/tmd 16.0 pair_coeff * * sw/mod 1 tmd.sw.mod Mo S S NULL NULL NULL - pair_coeff * * sw/mod 2 tmd.sw.mod NULL NULL NULL Mo S S - pair_coeff * * ilp/tmd MoS2.ILP Mo S S Mo S S + pair_coeff * * sw/mod 2 tmd.sw.mod NULL NULL NULL W Se Se + pair_coeff * * ilp/tmd TMD.ILP Mo S S W Se Se Description """"""""""" @@ -36,7 +36,7 @@ Description The *ilp/tmd* style computes the registry-dependent interlayer potential (ILP) potential for transition metal dichalcogenides (TMD) -as described in :ref:`(Ouyang7) `. +as described in :ref:`(Ouyang7) ` and ref:`(Jiang) `. .. math:: @@ -69,7 +69,7 @@ calculating the normals. each atom `i`, its six nearest neighboring atoms belonging to the same sub-layer are chosen to define the normal vector `{\bf n}_i`. -The parameter file (e.g. MoS2.ILP), is intended for use with *metal* +The parameter file (e.g. TMD.ILP), is intended for use with *metal* :doc:`units `, with energies in meV. Two additional parameters, *S*, and *rcut* are included in the parameter file. *S* is designed to facilitate scaling of energies. *rcut* is designed to build the neighbor @@ -77,7 +77,7 @@ list for calculating the normals for each atom pair. .. note:: - The parameters presented in the parameter file (e.g. MoS2.ILP), + The parameters presented in the parameter file (e.g. TMD.ILP), are fitted with taper function by setting the cutoff equal to 16.0 Angstrom. Using different cutoff or taper function should be careful. These parameters provide a good description in both short- and long-range @@ -133,10 +133,10 @@ if LAMMPS was built with that package. See the :doc:`Build package This pair style requires the newton setting to be *on* for pair interactions. -The MoS2.ILP potential file provided with LAMMPS (see the potentials +The TMD.ILP potential file provided with LAMMPS (see the potentials directory) are parameterized for *metal* units. You can use this potential with any LAMMPS units, but you would need to create your own -custom MoS2.ILP potential file with coefficients listed in the appropriate +custom TMD.ILP potential file with coefficients listed in the appropriate units, if your simulation does not use *metal* units. Related commands @@ -164,3 +164,7 @@ tap_flag = 1 .. _Ouyang7: **(Ouyang7)** W. Ouyang, et al., J. Chem. Theory Comput. 17, 7237 (2021). + +.. _Jiang: + +**(Jiang)** W. Jiang, et al., J. Phys. Chem. A, 127, 46, 9820–9830 (2023). From 20183ac9cc9680c608cb230a61c9474b3db8174b Mon Sep 17 00:00:00 2001 From: oywg11 Date: Mon, 18 Dec 2023 16:56:24 +0800 Subject: [PATCH 11/15] update potential file for ilp/tmd --- potentials/TMD.ILP | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 potentials/TMD.ILP diff --git a/potentials/TMD.ILP b/potentials/TMD.ILP new file mode 100644 index 0000000000..e7a9cbe558 --- /dev/null +++ b/potentials/TMD.ILP @@ -0,0 +1,25 @@ +# DATE: 2021-12-02 UNITS: metal CONTRIBUTOR: Wengen Ouyang w.g.ouyang@gmail.com +# CITATION: W. Ouyang, et al., J. Chem. Theory Comput. 17, 7237 (2021). +# CITATION: W. Jiang, et al., J. Phys. Chem. A, 127, 46, 9820–9830 (2023). +# Interlayer Potential (ILP) for bilayer and bulk Group-VI Transition Metal Dichalcogenides. +# The parameters below are fitted against the HSE + MBD-NL DFT reference data. +# +# -------------------- Repulsion Potential -------------------++++++++++++++++ Vdw Potential ++++++++++++++++********* +# beta(A) alpha delta(A) epsilon(meV) C(meV) d sR reff(A) C6(meV*A^6) S rcut +Mo Mo 5.579450 9.377662 2.027222 144.151775 97.978570 89.437597 2.059031 5.122055 491850.316195 1.0 4.0 +W W 5.530854 6.624992 1.983208 0.271792 140.174059 107.392585 1.356333 4.437591 691850.243962 1.0 4.0 +S S 3.161402 8.093263 1.953140 4.586764 118.065466 58.809416 0.215367 4.299600 148811.243409 1.0 4.0 +Se Se 3.938627 10.515924 2.415783 3.012583 22.400612 116.864517 0.151121 5.884241 112506.195626 1.0 4.0 +Mo W 5.412298 8.647128 2.108665 51.177950 184.342860 201.281256 2.547743 2.492287 99996.913401 1.0 4.0 +Mo S 3.627152 19.971375 7.585031 76.101931 3.317496 45.720328 0.947470 4.410425 150597.857716 1.0 4.0 +Mo Se 6.196447 4.844134 14.362005 7.407221 0.058823 27.156223 0.976771 3.979186 786029.840651 1.0 4.0 +W S 3.680136 11.163004 32.254117 110.019679 79.381335 138.340438 0.900750 8.875776 250600.809034 1.0 4.0 +W Se 3.559392 20.638856 1.202717 20.478669 197.422484 10.005271 1.052738 3.815817 288321.561114 1.0 4.0 +S Se 2.820092 7.491151 1.933323 141.532559 293.127817 90.470904 0.390492 4.170885 117688.987069 1.0 4.0 +# Symmetric Atom Pair +W Mo 5.412298 8.647128 2.108665 51.177950 184.342860 201.281256 2.547743 2.492287 99996.913401 1.0 4.0 +S Mo 3.627152 19.971375 7.585031 76.101931 3.317496 45.720328 0.947470 4.410425 150597.857716 1.0 4.0 +Se Mo 6.196447 4.844134 14.362005 7.407221 0.058823 27.156223 0.976771 3.979186 786029.840651 1.0 4.0 +S W 3.680136 11.163004 32.254117 110.019679 79.381335 138.340438 0.900750 8.875776 250600.809034 1.0 4.0 +Se W 3.559392 20.638856 1.202717 20.478669 197.422484 10.005271 1.052738 3.815817 288321.561114 1.0 4.0 +Se S 2.820092 7.491151 1.933323 141.532559 293.127817 90.470904 0.390492 4.170885 117688.987069 1.0 4.0 From 40f27eb7cf6cecc39be6090b07e66d96e7f68263 Mon Sep 17 00:00:00 2001 From: oywg11 Date: Mon, 18 Dec 2023 17:36:04 +0800 Subject: [PATCH 12/15] update doc file for ilp/tmd --- doc/src/pair_ilp_tmd.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/src/pair_ilp_tmd.rst b/doc/src/pair_ilp_tmd.rst index dcacc18ed6..b5d9ad60b5 100644 --- a/doc/src/pair_ilp_tmd.rst +++ b/doc/src/pair_ilp_tmd.rst @@ -36,7 +36,7 @@ Description The *ilp/tmd* style computes the registry-dependent interlayer potential (ILP) potential for transition metal dichalcogenides (TMD) -as described in :ref:`(Ouyang7) ` and ref:`(Jiang) `. +as described in :ref:`(Ouyang7) ` and :ref:`(Jiang) `. .. math:: From af222711aedc10d95baa35541e222d40b94c7605 Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Mon, 18 Dec 2023 15:52:30 -0600 Subject: [PATCH 13/15] Added text to explain the output of ctest -V a bit more --- doc/src/Developer_unittest.rst | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/doc/src/Developer_unittest.rst b/doc/src/Developer_unittest.rst index c0cdee6c09..5cb7a0ebde 100644 --- a/doc/src/Developer_unittest.rst +++ b/doc/src/Developer_unittest.rst @@ -569,6 +569,10 @@ For example, the following output snippet shows the failed unit test Expected: (err) <= (epsilon) Actual: 0.00022892713393549854 vs 0.0001 +The failed assertions provide line numbers in the test source +(e.g. ``test_main.cpp:56``), from which one can understand what +specific assertion failed. + Note that the force style engine runs one of a small number of systems in a rather off-equilibrium configuration with a few atoms for a few steps, writes data and restart files, uses :doc:`the clear command @@ -606,15 +610,18 @@ and run the test with verbose output. For example, env TEST_ARGS=-v ctest -R ^MolPairStyle:lj_cut_coul_long -V +``ctest`` with the ``-V`` flag also shows the exact command line +of the test. One can then use ``gdb --args`` to furthere debug and +catch exceptions with the test command, for example, + +.. code-block:: bash + + gdb --args /path/to/lammps/build/test_pair_style "/path/to/lammps/unittest/force-styles/tests/mol-pair-lj_cut_coul_long.yaml" + + It is recommended to configure the build with ``-D BUILD_SHARED_LIBS=on`` and use a custom linker to shorten the build time during recompilation. Installing `ccache` in your development environment helps speed up recompilation by caching previous compilations and detecting when the same compilation is being done again. Please see :doc:`Build_development` for further details. - - - - - - From 6c798412b459079b1d0275b3147c480d743b17ed Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Mon, 18 Dec 2023 18:52:42 -0500 Subject: [PATCH 14/15] fix typo and remove unnecessary quotes --- doc/src/Developer_unittest.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/src/Developer_unittest.rst b/doc/src/Developer_unittest.rst index 5cb7a0ebde..b48c3b4e17 100644 --- a/doc/src/Developer_unittest.rst +++ b/doc/src/Developer_unittest.rst @@ -611,12 +611,12 @@ and run the test with verbose output. For example, env TEST_ARGS=-v ctest -R ^MolPairStyle:lj_cut_coul_long -V ``ctest`` with the ``-V`` flag also shows the exact command line -of the test. One can then use ``gdb --args`` to furthere debug and +of the test. One can then use ``gdb --args`` to further debug and catch exceptions with the test command, for example, .. code-block:: bash - gdb --args /path/to/lammps/build/test_pair_style "/path/to/lammps/unittest/force-styles/tests/mol-pair-lj_cut_coul_long.yaml" + gdb --args /path/to/lammps/build/test_pair_style /path/to/lammps/unittest/force-styles/tests/mol-pair-lj_cut_coul_long.yaml It is recommended to configure the build with ``-D From e98df7018b5e73fa86bfb2ffce1077d5781fb3e7 Mon Sep 17 00:00:00 2001 From: oywg11 Date: Tue, 19 Dec 2023 08:17:43 +0800 Subject: [PATCH 15/15] update the example files --- examples/PACKAGES/interlayer/ilp_tmds/MoS2.ILP | 1 - examples/PACKAGES/interlayer/ilp_tmds/TMD.ILP | 1 + examples/PACKAGES/interlayer/ilp_tmds/in.mos2 | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) delete mode 120000 examples/PACKAGES/interlayer/ilp_tmds/MoS2.ILP create mode 120000 examples/PACKAGES/interlayer/ilp_tmds/TMD.ILP diff --git a/examples/PACKAGES/interlayer/ilp_tmds/MoS2.ILP b/examples/PACKAGES/interlayer/ilp_tmds/MoS2.ILP deleted file mode 120000 index 75dd894eef..0000000000 --- a/examples/PACKAGES/interlayer/ilp_tmds/MoS2.ILP +++ /dev/null @@ -1 +0,0 @@ -../../../../potentials/MoS2.ILP \ No newline at end of file diff --git a/examples/PACKAGES/interlayer/ilp_tmds/TMD.ILP b/examples/PACKAGES/interlayer/ilp_tmds/TMD.ILP new file mode 120000 index 0000000000..70f7ea18df --- /dev/null +++ b/examples/PACKAGES/interlayer/ilp_tmds/TMD.ILP @@ -0,0 +1 @@ +../../../../potentials/TMD.ILP \ No newline at end of file diff --git a/examples/PACKAGES/interlayer/ilp_tmds/in.mos2 b/examples/PACKAGES/interlayer/ilp_tmds/in.mos2 index b77f2fe719..0db4ec12d5 100644 --- a/examples/PACKAGES/interlayer/ilp_tmds/in.mos2 +++ b/examples/PACKAGES/interlayer/ilp_tmds/in.mos2 @@ -12,7 +12,7 @@ mass 4 95.94 pair_style hybrid/overlay sw/mod sw/mod ilp/tmd 16.0 pair_coeff * * sw/mod 1 tmd.sw.mod Mo S S NULL NULL NULL pair_coeff * * sw/mod 2 tmd.sw.mod NULL NULL NULL Mo S S -pair_coeff * * ilp/tmd MoS2.ILP Mo S S Mo S S +pair_coeff * * ilp/tmd TMD.ILP Mo S S Mo S S # Calculate the pair potential compute 0 all pair ilp/tmd