From 8c50f56548da2b848451d9df2427271773747495 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Fri, 23 Apr 2021 21:18:07 -0400 Subject: [PATCH 01/24] add unit test for TextFileReader class --- unittest/commands/test_variables.cpp | 2 +- unittest/formats/CMakeLists.txt | 5 + unittest/formats/test_text_file_reader.cpp | 148 +++++++++++++++++++++ 3 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 unittest/formats/test_text_file_reader.cpp diff --git a/unittest/commands/test_variables.cpp b/unittest/commands/test_variables.cpp index f31959c3ff..663a2a78a1 100644 --- a/unittest/commands/test_variables.cpp +++ b/unittest/commands/test_variables.cpp @@ -105,8 +105,8 @@ protected: "# comments only\n five\n#END\n", fp); fclose(fp); - fp = fopen("test_variable.atomfile", "w"); + fp = fopen("test_variable.atomfile", "w"); fputs("# test file for atomfile style variable\n\n" "4 # four lines\n4 0.5 #with comment\n" "2 -0.5 \n3 1.5\n1 -1.5\n\n" diff --git a/unittest/formats/CMakeLists.txt b/unittest/formats/CMakeLists.txt index 4c6de98729..b4c637edfb 100644 --- a/unittest/formats/CMakeLists.txt +++ b/unittest/formats/CMakeLists.txt @@ -28,6 +28,11 @@ if(PKG_MANYBODY) set_tests_properties(EIMPotentialFileReader PROPERTIES ENVIRONMENT "LAMMPS_POTENTIALS=${LAMMPS_POTENTIALS_DIR}") endif() +add_executable(test_text_file_reader test_text_file_reader.cpp) +target_link_libraries(test_text_file_reader PRIVATE lammps GTest::GMock GTest::GTest) +add_test(NAME TextFileReader COMMAND test_text_file_reader WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) +set_tests_properties(TextFileReader PROPERTIES ENVIRONMENT "LAMMPS_POTENTIALS=${LAMMPS_POTENTIALS_DIR}") + add_executable(test_file_operations test_file_operations.cpp) target_link_libraries(test_file_operations PRIVATE lammps GTest::GMock GTest::GTest) add_test(NAME FileOperations COMMAND test_file_operations WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) diff --git a/unittest/formats/test_text_file_reader.cpp b/unittest/formats/test_text_file_reader.cpp new file mode 100644 index 0000000000..e0bb2d42b5 --- /dev/null +++ b/unittest/formats/test_text_file_reader.cpp @@ -0,0 +1,148 @@ +/* ---------------------------------------------------------------------- + LAMMPS - Large-scale Atomic/Molecular Massively Parallel Simulator + https://lammps.sandia.gov/, Sandia National Laboratories + Steve Plimpton, sjplimp@sandia.gov + + Copyright (2003) Sandia Corporation. Under the terms of Contract + DE-AC04-94AL85000 with Sandia Corporation, the U.S. Government retains + certain rights in this software. This software is distributed under + the GNU General Public License. + + See the README file in the top-level LAMMPS directory. +------------------------------------------------------------------------- */ + +#include "info.h" +#include "input.h" +#include "text_file_reader.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +#include "../testing/core.h" + +#include +#include +#include +#include + +using namespace LAMMPS_NS; +using LAMMPS_NS::utils::split_words; + +// whether to print verbose output (i.e. not capturing LAMMPS screen output). +bool verbose = false; + +class TextFileReaderTest : public ::testing::Test { + +protected: + void TearDown() override + { + unlink("text_reader_one.file"); + unlink("text_reader_two.file"); + } + + void test_files() + { + FILE *fp = fopen("text_reader_one.file", "w"); + fputs("# test file 1 for text file reader\n\n\none\n two \n\n" + "three # with comment\nfour ! with non-comment\n" + "# comments only\n five\n#END\n", + fp); + fclose(fp); + + fp = fopen("text_reader_two.file", "w"); + fputs("# test file for atomfile style variable\n\n" + "4 # four lines\n4 0.5 #with comment\n" + "2 -0.5 \n3 1.5\n1 -1.5\n\n" + "2\n10 1.0 # test\n13 1.0\n\n######\n" + "4\n1 4.0 # test\n2 3.0\n3 2.0\n4 1.0\n#END\n", + fp); + fclose(fp); + } +}; + +TEST_F(TextFileReaderTest, nofile) +{ + ASSERT_THROW({ TextFileReader reader("text_reader_noexist.file", "test"); }, + FileReaderException); +} + +TEST_F(TextFileReaderTest, permissions) +{ + FILE *fp = fopen("text_reader_noperms.file", "w"); + fputs("word\n", fp); + fclose(fp); + chmod("text_reader_noperms.file", 0); + ASSERT_THROW({ TextFileReader reader("text_reader_noperms.file", "test"); }, + FileReaderException); + unlink("text_reader_noperms.file"); +} + +TEST_F(TextFileReaderTest, comments) +{ + test_files(); + TextFileReader reader("text_reader_two.file", "test"); + reader.ignore_comments = true; + auto line = reader.next_line(); + ASSERT_STREQ(line, "4 "); + line = reader.next_line(1); + ASSERT_STREQ(line, "4 0.5 "); + ASSERT_NO_THROW({ reader.skip_line(); }); + auto values = reader.next_values(1); + ASSERT_EQ(values.count(), 2); + ASSERT_EQ(values.next_int(), 3); + ASSERT_STREQ(values.next_string().c_str(), "1.5"); + ASSERT_NE(reader.next_line(), nullptr); + double data[20]; + ASSERT_THROW({ reader.next_dvector(data,20); }, FileReaderException); + ASSERT_THROW({ reader.skip_line(); }, EOFException); + ASSERT_EQ(reader.next_line(), nullptr); +} + +TEST_F(TextFileReaderTest, nocomments) +{ + test_files(); + TextFileReader reader("text_reader_one.file", "test"); + reader.ignore_comments = false; + auto line = reader.next_line(); + ASSERT_STREQ(line, "# test file 1 for text file reader\n"); + line = reader.next_line(1); + ASSERT_STREQ(line, "one\n"); + ASSERT_NO_THROW({ reader.skip_line(); }); + auto values = reader.next_values(4); + ASSERT_EQ(values.count(), 4); + ASSERT_STREQ(values.next_string().c_str(), "three"); + ASSERT_STREQ(values.next_string().c_str(), "#"); + ASSERT_STREQ(values.next_string().c_str(), "with"); + try { + reader.next_values(100); + FAIL() << "No exception thrown\n"; + } catch (EOFException &e) { + ASSERT_STREQ(e.what(), "Incorrect format in test file! 9/100 parameters"); + } + ASSERT_THROW({ reader.skip_line(); }, EOFException); + ASSERT_EQ(reader.next_line(), nullptr); +} + +int main(int argc, char **argv) +{ + MPI_Init(&argc, &argv); + ::testing::InitGoogleMock(&argc, argv); + + if (Info::get_mpi_vendor() == "Open MPI" && !LAMMPS_NS::Info::has_exceptions()) + std::cout << "Warning: using OpenMPI without exceptions. " + "Death tests will be skipped\n"; + + // handle arguments passed via environment variable + if (const char *var = getenv("TEST_ARGS")) { + std::vector env = split_words(var); + for (auto arg : env) { + if (arg == "-v") { + verbose = true; + } + } + } + if ((argc > 1) && (strcmp(argv[1], "-v") == 0)) verbose = true; + + int rv = RUN_ALL_TESTS(); + MPI_Finalize(); + return rv; +} From cf81f72aadb68430ca34f73e302f086ad4cfdde9 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 24 Apr 2021 01:22:06 -0400 Subject: [PATCH 02/24] more tests for tokenizer classes --- unittest/utils/test_tokenizer.cpp | 44 ++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/unittest/utils/test_tokenizer.cpp b/unittest/utils/test_tokenizer.cpp index ec097abf62..7852abb632 100644 --- a/unittest/utils/test_tokenizer.cpp +++ b/unittest/utils/test_tokenizer.cpp @@ -53,6 +53,11 @@ TEST(Tokenizer, skip) ASSERT_FALSE(t.has_next()); ASSERT_EQ(t.count(), 2); ASSERT_THROW(t.skip(), TokenizerException); + try { + t.skip(); + } catch (TokenizerException &e) { + ASSERT_STREQ(e.what(), "No more tokens"); + } } TEST(Tokenizer, prefix_separators) @@ -87,6 +92,15 @@ TEST(Tokenizer, copy_constructor) ASSERT_EQ(u.count(), 2); } +TEST(Tokenizer, rvalue) +{ + auto u = Tokenizer(" test new word ", " "); + ASSERT_THAT(u.next(), Eq("test")); + ASSERT_THAT(u.next(), Eq("new")); + ASSERT_THAT(u.next(), Eq("word")); + ASSERT_EQ(u.count(), 3); +} + TEST(Tokenizer, no_separator_path) { Tokenizer t("one", ":"); @@ -181,12 +195,40 @@ TEST(ValueTokenizer, skip) ASSERT_FALSE(t.has_next()); ASSERT_EQ(t.count(), 2); ASSERT_THROW(t.skip(), TokenizerException); + try { + t.skip(); + } catch (TokenizerException &e) { + ASSERT_STREQ(e.what(), "No more tokens"); + } +} + +TEST(ValueTokenizer, copy_constructor) +{ + ValueTokenizer t(" test word ", " "); + ASSERT_THAT(t.next_string(), Eq("test")); + ASSERT_THAT(t.next_string(), Eq("word")); + ASSERT_EQ(t.count(), 2); + ValueTokenizer u(t); + ASSERT_THAT(u.next_string(), Eq("test")); + ASSERT_THAT(u.next_string(), Eq("word")); + ASSERT_EQ(u.count(), 2); +} + +TEST(ValueTokenizer, rvalue) +{ + auto u = ValueTokenizer(" test new word ", " "); + ASSERT_THAT(u.next_string(), Eq("test")); + ASSERT_THAT(u.next_string(), Eq("new")); + ASSERT_THAT(u.next_string(), Eq("word")); + ASSERT_EQ(u.count(), 3); } TEST(ValueTokenizer, bad_integer) { - ValueTokenizer values("f10"); + ValueTokenizer values("f10 f11 f12"); ASSERT_THROW(values.next_int(), InvalidIntegerException); + ASSERT_THROW(values.next_bigint(), InvalidIntegerException); + ASSERT_THROW(values.next_tagint(), InvalidIntegerException); } TEST(ValueTokenizer, bad_double) From 6a9b44133107f99b4b4999c3de5a6de4102f1443 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 24 Apr 2021 01:22:23 -0400 Subject: [PATCH 03/24] add tests for writing restart files --- unittest/formats/test_file_operations.cpp | 109 +++++++++++++++++++++- 1 file changed, 107 insertions(+), 2 deletions(-) diff --git a/unittest/formats/test_file_operations.cpp b/unittest/formats/test_file_operations.cpp index 700990fb72..27f45b69fc 100644 --- a/unittest/formats/test_file_operations.cpp +++ b/unittest/formats/test_file_operations.cpp @@ -11,13 +11,15 @@ See the README file in the top-level LAMMPS directory. ------------------------------------------------------------------------- */ +#include "../testing/core.h" +#include "atom.h" +#include "domain.h" #include "info.h" #include "input.h" #include "lammps.h" -#include "utils.h" +#include "update.h" #include "gmock/gmock.h" #include "gtest/gtest.h" -#include "../testing/core.h" #include #include @@ -57,6 +59,13 @@ protected: LAMMPSTest::TearDown(); remove("safe_file_read_test.txt"); } + + bool file_exists(const char *file) + { + FILE *fp = fopen(file, "r"); + fclose(fp); + return fp ? true : false; + } }; #define MAX_BUF_SIZE 128 @@ -145,6 +154,102 @@ TEST_F(FileOperationsTest, logmesg) remove("test_logmesg.log"); } +TEST_F(FileOperationsTest, restart) +{ + BEGIN_HIDE_OUTPUT(); + command("echo none"); + END_HIDE_OUTPUT(); + TEST_FAILURE(".*ERROR: Write_restart command before simulation box is defined.*", + command("write_restart test.restart");); + + BEGIN_HIDE_OUTPUT(); + command("region box block -2 2 -2 2 -2 2"); + command("create_box 1 box"); + command("create_atoms 1 single 0.0 0.0 0.0"); + command("mass 1 1.0"); + command("reset_timestep 333"); + command("comm_modify cutoff 0.2"); + command("write_restart noinit.restart noinit"); + command("run 0 post no"); + command("write_restart test.restart"); + command("write_restart step*.restart"); + command("write_restart multi-%.restart"); + command("write_restart multi2-%.restart fileper 2"); + command("write_restart multi3-%.restart nfile 1"); + if (info->has_package("MPIIO")) command("write_restart test.restart.mpiio"); + END_HIDE_OUTPUT(); + + ASSERT_TRUE(file_exists("noinit.restart")); + ASSERT_TRUE(file_exists("test.restart")); + ASSERT_TRUE(file_exists("step333.restart")); + ASSERT_TRUE(file_exists("multi-base.restart")); + ASSERT_TRUE(file_exists("multi-0.restart")); + ASSERT_TRUE(file_exists("multi2-base.restart")); + ASSERT_TRUE(file_exists("multi2-0.restart")); + ASSERT_TRUE(file_exists("multi3-base.restart")); + ASSERT_TRUE(file_exists("multi3-0.restart")); + if (info->has_package("MPIIO")) ASSERT_TRUE(file_exists("test.restart.mpiio")); + + if (!info->has_package("MPIIO")) { + TEST_FAILURE(".*ERROR: Illegal write_restart command.*", + command("write_restart test.restart.mpiio");); + } else { + TEST_FAILURE(".*ERROR: Restart file MPI-IO output not allowed with % in filename.*", + command("write_restart test.restart-%.mpiio");); + } + + TEST_FAILURE(".*ERROR: Illegal write_restart command.*", command("write_restart");); + TEST_FAILURE(".*ERROR: Illegal write_restart command.*", + command("write_restart test.restart xxxx");); + TEST_FAILURE(".*ERROR on proc 0: Cannot open restart file some_crazy_dir/test.restart:" + " No such file or directory.*", + command("write_restart some_crazy_dir/test.restart");); + BEGIN_HIDE_OUTPUT(); + command("clear"); + END_HIDE_OUTPUT(); + ASSERT_EQ(lmp->atom->natoms, 0); + ASSERT_EQ(lmp->update->ntimestep, 0); + ASSERT_EQ(lmp->domain->triclinic, 0); + + TEST_FAILURE( + ".*ERROR on proc 0: Cannot open restart file noexist.restart: No such file or directory.*", + command("read_restart noexist.restart");); + + BEGIN_HIDE_OUTPUT(); + command("read_restart step333.restart"); + command("change_box all triclinic"); + command("write_restart triclinic.restart"); + END_HIDE_OUTPUT(); + ASSERT_EQ(lmp->atom->natoms, 1); + ASSERT_EQ(lmp->update->ntimestep, 333); + ASSERT_EQ(lmp->domain->triclinic, 1); + BEGIN_HIDE_OUTPUT(); + command("clear"); + END_HIDE_OUTPUT(); + ASSERT_EQ(lmp->atom->natoms, 0); + ASSERT_EQ(lmp->update->ntimestep, 0); + ASSERT_EQ(lmp->domain->triclinic, 0); + BEGIN_HIDE_OUTPUT(); + command("read_restart triclinic.restart"); + END_HIDE_OUTPUT(); + ASSERT_EQ(lmp->atom->natoms, 1); + ASSERT_EQ(lmp->update->ntimestep, 333); + ASSERT_EQ(lmp->domain->triclinic, 1); + + // clean up + unlink("noinit.restart"); + unlink("test.restart"); + unlink("step333.restart"); + unlink("multi-base.restart"); + unlink("multi-0.restart"); + unlink("multi2-base.restart"); + unlink("multi2-0.restart"); + unlink("multi3-base.restart"); + unlink("multi3-0.restart"); + unlink("triclinic.restart"); + if (info->has_package("MPIIO")) unlink("test.restart.mpiio"); +} + int main(int argc, char **argv) { MPI_Init(&argc, &argv); From 6943a3da354717404c7dfc6cf242c9e122a79656 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 24 Apr 2021 07:12:45 -0400 Subject: [PATCH 04/24] must check if file is readable before changes to internal data --- src/read_data.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/read_data.cpp b/src/read_data.cpp index c0085f19d1..c8937cbfc9 100644 --- a/src/read_data.cpp +++ b/src/read_data.cpp @@ -304,6 +304,12 @@ void ReadData::command(int narg, char **arg) extra_dihedral_types || extra_improper_types)) error->all(FLERR,"Cannot use read_data extra with add flag"); + // check if data file is available and readable + + if (!utils::file_is_readable(arg[0])) + error->all(FLERR,fmt::format("Cannot open file {}: {}", + arg[0], utils::getsyserror())); + // first time system initialization if (addflag == NONE) { From 9e7d26351d68c7a2d8279a05961b6a1056cd153e Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 24 Apr 2021 07:13:06 -0400 Subject: [PATCH 05/24] tweak epsilon for GPU package tests --- unittest/force-styles/tests/atomic-pair-yukawa_colloid.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittest/force-styles/tests/atomic-pair-yukawa_colloid.yaml b/unittest/force-styles/tests/atomic-pair-yukawa_colloid.yaml index 1529cf51b9..1c5c24832d 100644 --- a/unittest/force-styles/tests/atomic-pair-yukawa_colloid.yaml +++ b/unittest/force-styles/tests/atomic-pair-yukawa_colloid.yaml @@ -1,7 +1,7 @@ --- lammps_version: 10 Feb 2021 date_generated: Fri Feb 26 23:09:10 2021 -epsilon: 5e-14 +epsilon: 2e-13 prerequisites: ! | atom sphere pair yukawa/colloid From 2c4017d3ace7317ea5e17cd435e2ca98ed021940 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 24 Apr 2021 07:13:26 -0400 Subject: [PATCH 06/24] add test for write_dump cfg --- unittest/formats/test_dump_cfg.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/unittest/formats/test_dump_cfg.cpp b/unittest/formats/test_dump_cfg.cpp index b8f879de6f..fb404e07b5 100644 --- a/unittest/formats/test_dump_cfg.cpp +++ b/unittest/formats/test_dump_cfg.cpp @@ -75,6 +75,29 @@ TEST_F(DumpCfgTest, run0) delete_file("dump_cfg_run0.melt.cfg"); } +TEST_F(DumpCfgTest, write_dump) +{ + auto dump_file = "dump_cfg_run*.melt.cfg"; + auto fields = "mass type xs ys zs id proc procp1 x y z ix iy iz vx vy vz fx fy fz"; + + BEGIN_HIDE_OUTPUT(); + command(std::string("write_dump all cfg dump_cfg.melt.cfg ") + fields); + command(std::string("write_dump all cfg dump_cfg*.melt.cfg ") + fields); + END_HIDE_OUTPUT(); + + ASSERT_FILE_EXISTS("dump_cfg.melt.cfg"); + auto lines = read_lines("dump_cfg.melt.cfg"); + ASSERT_EQ(lines.size(), 124); + ASSERT_THAT(lines[0], Eq("Number of particles = 32")); + delete_file("dump_cfg.melt.cfg"); + + ASSERT_FILE_EXISTS("dump_cfg0.melt.cfg"); + lines = read_lines("dump_cfg0.melt.cfg"); + ASSERT_EQ(lines.size(), 124); + ASSERT_THAT(lines[0], Eq("Number of particles = 32")); + delete_file("dump_cfg0.melt.cfg"); +} + TEST_F(DumpCfgTest, unwrap_run0) { auto dump_file = "dump_cfg_unwrap_run*.melt.cfg"; From e980d178820e5ed642e08ca30a1562e7ead951bb Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 24 Apr 2021 07:14:04 -0400 Subject: [PATCH 07/24] reuse existing code. add tests for write_data --- unittest/formats/test_file_operations.cpp | 155 ++++++++++++++++------ 1 file changed, 117 insertions(+), 38 deletions(-) diff --git a/unittest/formats/test_file_operations.cpp b/unittest/formats/test_file_operations.cpp index 27f45b69fc..7683f712a7 100644 --- a/unittest/formats/test_file_operations.cpp +++ b/unittest/formats/test_file_operations.cpp @@ -12,6 +12,7 @@ ------------------------------------------------------------------------- */ #include "../testing/core.h" +#include "../testing/utils.h" #include "atom.h" #include "domain.h" #include "info.h" @@ -45,13 +46,10 @@ protected: LAMMPSTest::SetUp(); ASSERT_NE(lmp, nullptr); - FILE *fp = fopen("safe_file_read_test.txt", "wb"); - ASSERT_NE(fp, nullptr); - fputs("one line\n", fp); - fputs("two_lines\n", fp); - fputs("\n", fp); - fputs("no newline", fp); - fclose(fp); + std::ofstream out("safe_file_read_test.txt", std::ios_base::out | std::ios_base::binary); + ASSERT_TRUE(out.good()); + out << "one line\ntwo_lines\n\nno newline"; + out.close(); } void TearDown() override @@ -59,13 +57,6 @@ protected: LAMMPSTest::TearDown(); remove("safe_file_read_test.txt"); } - - bool file_exists(const char *file) - { - FILE *fp = fopen(file, "r"); - fclose(fp); - return fp ? true : false; - } }; #define MAX_BUF_SIZE 128 @@ -73,7 +64,7 @@ TEST_F(FileOperationsTest, safe_fgets) { char buf[MAX_BUF_SIZE]; - FILE *fp = fopen("safe_file_read_test.txt", "r"); + FILE *fp = fopen("safe_file_read_test.txt", "rb"); ASSERT_NE(fp, nullptr); memset(buf, 0, MAX_BUF_SIZE); @@ -109,7 +100,7 @@ TEST_F(FileOperationsTest, safe_fread) { char buf[MAX_BUF_SIZE]; - FILE *fp = fopen("safe_file_read_test.txt", "r"); + FILE *fp = fopen("safe_file_read_test.txt", "rb"); ASSERT_NE(fp, nullptr); memset(buf, 0, MAX_BUF_SIZE); @@ -154,7 +145,7 @@ TEST_F(FileOperationsTest, logmesg) remove("test_logmesg.log"); } -TEST_F(FileOperationsTest, restart) +TEST_F(FileOperationsTest, write_restart) { BEGIN_HIDE_OUTPUT(); command("echo none"); @@ -179,16 +170,16 @@ TEST_F(FileOperationsTest, restart) if (info->has_package("MPIIO")) command("write_restart test.restart.mpiio"); END_HIDE_OUTPUT(); - ASSERT_TRUE(file_exists("noinit.restart")); - ASSERT_TRUE(file_exists("test.restart")); - ASSERT_TRUE(file_exists("step333.restart")); - ASSERT_TRUE(file_exists("multi-base.restart")); - ASSERT_TRUE(file_exists("multi-0.restart")); - ASSERT_TRUE(file_exists("multi2-base.restart")); - ASSERT_TRUE(file_exists("multi2-0.restart")); - ASSERT_TRUE(file_exists("multi3-base.restart")); - ASSERT_TRUE(file_exists("multi3-0.restart")); - if (info->has_package("MPIIO")) ASSERT_TRUE(file_exists("test.restart.mpiio")); + ASSERT_FILE_EXISTS("noinit.restart"); + ASSERT_FILE_EXISTS("test.restart"); + ASSERT_FILE_EXISTS("step333.restart"); + ASSERT_FILE_EXISTS("multi-base.restart"); + ASSERT_FILE_EXISTS("multi-0.restart"); + ASSERT_FILE_EXISTS("multi2-base.restart"); + ASSERT_FILE_EXISTS("multi2-0.restart"); + ASSERT_FILE_EXISTS("multi3-base.restart"); + ASSERT_FILE_EXISTS("multi3-0.restart"); + if (info->has_package("MPIIO")) ASSERT_FILE_EXISTS("test.restart.mpiio"); if (!info->has_package("MPIIO")) { TEST_FAILURE(".*ERROR: Illegal write_restart command.*", @@ -237,17 +228,105 @@ TEST_F(FileOperationsTest, restart) ASSERT_EQ(lmp->domain->triclinic, 1); // clean up - unlink("noinit.restart"); - unlink("test.restart"); - unlink("step333.restart"); - unlink("multi-base.restart"); - unlink("multi-0.restart"); - unlink("multi2-base.restart"); - unlink("multi2-0.restart"); - unlink("multi3-base.restart"); - unlink("multi3-0.restart"); - unlink("triclinic.restart"); - if (info->has_package("MPIIO")) unlink("test.restart.mpiio"); + delete_file("noinit.restart"); + delete_file("test.restart"); + delete_file("step333.restart"); + delete_file("multi-base.restart"); + delete_file("multi-0.restart"); + delete_file("multi2-base.restart"); + delete_file("multi2-0.restart"); + delete_file("multi3-base.restart"); + delete_file("multi3-0.restart"); + delete_file("triclinic.restart"); + if (info->has_package("MPIIO")) delete_file("test.restart.mpiio"); +} + +TEST_F(FileOperationsTest, write_data) +{ + BEGIN_HIDE_OUTPUT(); + command("echo none"); + END_HIDE_OUTPUT(); + TEST_FAILURE(".*ERROR: Write_data command before simulation box is defined.*", + command("write_data test.data");); + + BEGIN_HIDE_OUTPUT(); + command("region box block -2 2 -2 2 -2 2"); + command("create_box 2 box"); + command("create_atoms 1 single 0.5 0.0 0.0"); + command("pair_style zero 1.0"); + command("pair_coeff * *"); + command("mass * 1.0"); + command("reset_timestep 333"); + command("write_data noinit.data"); + command("write_data nocoeff.data nocoeff"); + command("run 0 post no"); + command("write_data test.data"); + command("write_data step*.data pair ij"); + command("fix q all property/atom q"); + command("set type 1 charge -0.5"); + command("write_data charge.data"); + command("write_data nofix.data nofix"); + END_HIDE_OUTPUT(); + + ASSERT_FILE_EXISTS("noinit.data"); + ASSERT_EQ(count_lines("noinit.data"), 26); + ASSERT_FILE_EXISTS("test.data"); + ASSERT_EQ(count_lines("test.data"), 26); + ASSERT_FILE_EXISTS("step333.data"); + ASSERT_EQ(count_lines("step333.data"), 27); + ASSERT_FILE_EXISTS("nocoeff.data"); + ASSERT_EQ(count_lines("nocoeff.data"), 21); + ASSERT_FILE_EXISTS("nofix.data"); + ASSERT_EQ(count_lines("nofix.data"), 26); + ASSERT_FILE_EXISTS("charge.data"); + ASSERT_EQ(count_lines("charge.data"), 30); + + TEST_FAILURE(".*ERROR: Illegal write_data command.*", command("write_data");); + TEST_FAILURE(".*ERROR: Illegal write_data command.*", command("write_data test.data xxxx");); + TEST_FAILURE(".*ERROR on proc 0: Cannot open data file some_crazy_dir/test.data:" + " No such file or directory.*", + command("write_data some_crazy_dir/test.data");); + + BEGIN_HIDE_OUTPUT(); + command("clear"); + END_HIDE_OUTPUT(); + ASSERT_EQ(lmp->domain->box_exist, 0); + ASSERT_EQ(lmp->atom->natoms, 0); + ASSERT_EQ(lmp->update->ntimestep, 0); + ASSERT_EQ(lmp->domain->triclinic, 0); + + TEST_FAILURE(".*ERROR: Cannot open file noexist.data: No such file or directory.*", + command("read_data noexist.data");); + + BEGIN_HIDE_OUTPUT(); + command("pair_style zero 1.0"); + command("read_data step333.data"); + command("change_box all triclinic"); + command("write_data triclinic.data"); + END_HIDE_OUTPUT(); + ASSERT_EQ(lmp->atom->natoms, 1); + ASSERT_EQ(lmp->update->ntimestep, 0); + ASSERT_EQ(lmp->domain->triclinic, 1); + BEGIN_HIDE_OUTPUT(); + command("clear"); + END_HIDE_OUTPUT(); + ASSERT_EQ(lmp->atom->natoms, 0); + ASSERT_EQ(lmp->domain->triclinic, 0); + BEGIN_HIDE_OUTPUT(); + command("pair_style zero 1.0"); + command("read_data triclinic.data"); + END_HIDE_OUTPUT(); + ASSERT_EQ(lmp->atom->natoms, 1); + ASSERT_EQ(lmp->domain->triclinic, 1); + + // clean up + delete_file("charge.data"); + delete_file("nocoeff.data"); + delete_file("noinit.data"); + delete_file("nofix.data"); + delete_file("test.data"); + delete_file("step333.data"); + delete_file("triclinic.data"); } int main(int argc, char **argv) From 0aa64eaf14077343900351b1136e0e782857bb37 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 24 Apr 2021 07:14:29 -0400 Subject: [PATCH 08/24] portability improvement. replace POSIX-only functionality. --- unittest/testing/utils.h | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/unittest/testing/utils.h b/unittest/testing/utils.h index c76b37872a..730c6fdb44 100644 --- a/unittest/testing/utils.h +++ b/unittest/testing/utils.h @@ -10,14 +10,12 @@ See the README file in the top-level LAMMPS directory. ------------------------------------------------------------------------- */ -#ifndef TEST_EXTENSIONS__H -#define TEST_EXTENSIONS__H +#ifndef LMP_TESTING_UTILS_H +#define LMP_TESTING_UTILS_H #include #include #include -#include -#include #include static void delete_file(const std::string &filename) @@ -65,8 +63,8 @@ static std::vector read_lines(const std::string &filename) static bool file_exists(const std::string &filename) { - struct stat result; - return stat(filename.c_str(), &result) == 0; + std::ifstream infile(filename); + return infile.good(); } #define ASSERT_FILE_EXISTS(NAME) ASSERT_TRUE(file_exists(NAME)) From 66f690004d3ab2a7f55cdda5ac5b723935dfd9fc Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 24 Apr 2021 07:14:49 -0400 Subject: [PATCH 09/24] correctly test move constructors --- unittest/utils/test_tokenizer.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/unittest/utils/test_tokenizer.cpp b/unittest/utils/test_tokenizer.cpp index 7852abb632..5b20d24e7c 100644 --- a/unittest/utils/test_tokenizer.cpp +++ b/unittest/utils/test_tokenizer.cpp @@ -92,9 +92,9 @@ TEST(Tokenizer, copy_constructor) ASSERT_EQ(u.count(), 2); } -TEST(Tokenizer, rvalue) +TEST(Tokenizer, move_constructor) { - auto u = Tokenizer(" test new word ", " "); + Tokenizer u = std::move(Tokenizer("test new word ", " ")); ASSERT_THAT(u.next(), Eq("test")); ASSERT_THAT(u.next(), Eq("new")); ASSERT_THAT(u.next(), Eq("word")); @@ -214,9 +214,9 @@ TEST(ValueTokenizer, copy_constructor) ASSERT_EQ(u.count(), 2); } -TEST(ValueTokenizer, rvalue) +TEST(ValueTokenizer, move_constructor) { - auto u = ValueTokenizer(" test new word ", " "); + ValueTokenizer u = std::move(ValueTokenizer(" test new word ", " ")); ASSERT_THAT(u.next_string(), Eq("test")); ASSERT_THAT(u.next_string(), Eq("new")); ASSERT_THAT(u.next_string(), Eq("word")); From e6f57cdf2cb7ef9653c4b81d5dedde85fd834649 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 24 Apr 2021 07:21:29 -0400 Subject: [PATCH 10/24] minor tweaks --- unittest/formats/test_dump_cfg.cpp | 2 ++ unittest/formats/test_file_operations.cpp | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/unittest/formats/test_dump_cfg.cpp b/unittest/formats/test_dump_cfg.cpp index fb404e07b5..64930c3ac6 100644 --- a/unittest/formats/test_dump_cfg.cpp +++ b/unittest/formats/test_dump_cfg.cpp @@ -96,6 +96,8 @@ TEST_F(DumpCfgTest, write_dump) ASSERT_EQ(lines.size(), 124); ASSERT_THAT(lines[0], Eq("Number of particles = 32")); delete_file("dump_cfg0.melt.cfg"); + + TEST_FAILURE(".*ERROR: Unrecognized dump style 'xxx'.*", command("write_dump all xxx test.xxx");); } TEST_F(DumpCfgTest, unwrap_run0) diff --git a/unittest/formats/test_file_operations.cpp b/unittest/formats/test_file_operations.cpp index 7683f712a7..dbbad03adf 100644 --- a/unittest/formats/test_file_operations.cpp +++ b/unittest/formats/test_file_operations.cpp @@ -257,7 +257,7 @@ TEST_F(FileOperationsTest, write_data) command("pair_coeff * *"); command("mass * 1.0"); command("reset_timestep 333"); - command("write_data noinit.data"); + command("write_data noinit.data noinit"); command("write_data nocoeff.data nocoeff"); command("run 0 post no"); command("write_data test.data"); @@ -283,6 +283,7 @@ TEST_F(FileOperationsTest, write_data) TEST_FAILURE(".*ERROR: Illegal write_data command.*", command("write_data");); TEST_FAILURE(".*ERROR: Illegal write_data command.*", command("write_data test.data xxxx");); + TEST_FAILURE(".*ERROR: Illegal write_data command.*", command("write_data test.data pair xx");); TEST_FAILURE(".*ERROR on proc 0: Cannot open data file some_crazy_dir/test.data:" " No such file or directory.*", command("write_data some_crazy_dir/test.data");); From 43325dca82ac0d5dd4297697c0440e1b3737bfc2 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sun, 25 Apr 2021 00:19:22 -0400 Subject: [PATCH 11/24] update/add tests about starting up LAMMPS - move the test checking the help message from the c++ library to running the executable and checking the output - add a command line test for errors on invalid command line flags - add a c++ library test checking if ntreads is set to 1 without OMP_NUM_THREADS --- unittest/CMakeLists.txt | 16 +++++++ unittest/cplusplus/CMakeLists.txt | 1 + unittest/cplusplus/test_lammps_class.cpp | 59 ++++++++++++++++++++++-- 3 files changed, 71 insertions(+), 5 deletions(-) diff --git a/unittest/CMakeLists.txt b/unittest/CMakeLists.txt index addc8bc244..26db526b60 100644 --- a/unittest/CMakeLists.txt +++ b/unittest/CMakeLists.txt @@ -10,6 +10,22 @@ set_tests_properties(RunLammps PROPERTIES ENVIRONMENT "TSAN_OPTIONS=ignore_noninstrumented_modules=1" PASS_REGULAR_EXPRESSION "^LAMMPS \\([0-9]+ [A-Za-z]+ 2[0-9][0-9][0-9]\\)") +# check if the compiled executable will print the help message +add_test(NAME HelpMessage + COMMAND $ -h + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) +set_tests_properties(HelpMessage PROPERTIES + ENVIRONMENT "TSAN_OPTIONS=ignore_noninstrumented_modules=1;PAGER=head" + PASS_REGULAR_EXPRESSION ".*Large-scale Atomic/Molecular Massively Parallel Simulator -.*Usage example:.*") + +# check if the compiled executable will error out on an invalid command line flag +add_test(NAME InvalidFlag + COMMAND $ -xxx + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) +set_tests_properties(InvalidFlag PROPERTIES + ENVIRONMENT "TSAN_OPTIONS=ignore_noninstrumented_modules=1" + PASS_REGULAR_EXPRESSION "ERROR: Invalid command-line argument.*") + if(BUILD_MPI) function(add_mpi_test) set(MPI_TEST_NUM_PROCS 1) diff --git a/unittest/cplusplus/CMakeLists.txt b/unittest/cplusplus/CMakeLists.txt index 90ef9a0282..aef69f3722 100644 --- a/unittest/cplusplus/CMakeLists.txt +++ b/unittest/cplusplus/CMakeLists.txt @@ -2,6 +2,7 @@ add_executable(test_lammps_class test_lammps_class.cpp) target_link_libraries(test_lammps_class PRIVATE lammps GTest::GMockMain GTest::GTest GTest::GMock) add_test(LammpsClass test_lammps_class) +set_tests_properties(LammpsClass PROPERTIES ENVIRONMENT "OMP_NUM_THREADS=1") add_executable(test_input_class test_input_class.cpp) target_link_libraries(test_input_class PRIVATE lammps GTest::GTest GTest::GTestMain) diff --git a/unittest/cplusplus/test_lammps_class.cpp b/unittest/cplusplus/test_lammps_class.cpp index c20cdf336a..6a0e6719cc 100644 --- a/unittest/cplusplus/test_lammps_class.cpp +++ b/unittest/cplusplus/test_lammps_class.cpp @@ -1,13 +1,17 @@ // unit tests for the LAMMPS base class +#include "comm.h" +#include "info.h" #include "lammps.h" -#include // for stdin, stdout +#include // for stdin, stdout +#include // for setenv #include #include #include "gmock/gmock.h" #include "gtest/gtest.h" +using ::testing::MatchesRegex; using ::testing::StartsWith; namespace LAMMPS_NS { @@ -95,6 +99,7 @@ TEST_F(LAMMPS_plain, InitMembers) EXPECT_STREQ(LAMMPS::git_branch, "(unknown)"); EXPECT_STREQ(LAMMPS::git_descriptor, "(unknown)"); } + EXPECT_EQ(lmp->comm->nthreads, 1); } TEST_F(LAMMPS_plain, TestStyles) @@ -229,6 +234,7 @@ TEST_F(LAMMPS_omp, InitMembers) EXPECT_STREQ(LAMMPS::git_branch, "(unknown)"); EXPECT_STREQ(LAMMPS::git_descriptor, "(unknown)"); } + EXPECT_EQ(lmp->comm->nthreads, 2); } // test fixture for Kokkos tests @@ -318,10 +324,49 @@ TEST_F(LAMMPS_kokkos, InitMembers) } } -// check help message printing -TEST(LAMMPS_help, HelpMessage) +// check if Comm::nthreads is initialized to either 1 or 2 (from the previous tests) +TEST(LAMMPS_init, OpenMP) { - const char *args[] = {"LAMMPS_test", "-h"}; + if (!LAMMPS::is_installed_pkg("USER-OMP")) GTEST_SKIP(); + + FILE *fp = fopen("in.lammps_empty", "w"); + fputs("\n", fp); + fclose(fp); + + const char *args[] = {"LAMMPS_init", "-in", "in.lammps_empty", "-log", "none", "-nocite"}; + char **argv = (char **)args; + int argc = sizeof(args) / sizeof(char *); + + ::testing::internal::CaptureStdout(); + LAMMPS *lmp = new LAMMPS(argc, argv, MPI_COMM_WORLD); + std::string output = ::testing::internal::GetCapturedStdout(); + EXPECT_THAT(output, MatchesRegex(".*using 2 OpenMP thread.*per MPI task.*")); + + if (LAMMPS_NS::Info::has_accelerator_feature("USER-OMP", "api", "openmp")) + EXPECT_EQ(lmp->comm->nthreads, 2); + else + EXPECT_EQ(lmp->comm->nthreads, 1); + ::testing::internal::CaptureStdout(); + delete lmp; + ::testing::internal::GetCapturedStdout(); + + remove("in.lammps_empty"); +} + +// check no OMP_NUM_THREADS warning message printing. this must be the +// last OpenMP related test as threads will be locked to 1 from here on. + +TEST(LAMMPS_init, NoOpenMP) +{ + if (!LAMMPS_NS::Info::has_accelerator_feature("USER-OMP", "api", "openmp")) + GTEST_SKIP() << "No threading enabled"; + + FILE *fp = fopen("in.lammps_class_noomp", "w"); + fputs("\n", fp); + fclose(fp); + unsetenv("OMP_NUM_THREADS"); + + const char *args[] = {"LAMMPS_init", "-in", "in.lammps_class_noomp", "-log", "none", "-nocite"}; char **argv = (char **)args; int argc = sizeof(args) / sizeof(char *); @@ -329,7 +374,11 @@ TEST(LAMMPS_help, HelpMessage) LAMMPS *lmp = new LAMMPS(argc, argv, MPI_COMM_WORLD); std::string output = ::testing::internal::GetCapturedStdout(); EXPECT_THAT(output, - StartsWith("\nLarge-scale Atomic/Molecular Massively Parallel Simulator -")); + MatchesRegex(".*OMP_NUM_THREADS environment is not set.*Defaulting to 1 thread.*")); + EXPECT_EQ(lmp->comm->nthreads, 1); + ::testing::internal::CaptureStdout(); delete lmp; + ::testing::internal::GetCapturedStdout(); } + } // namespace LAMMPS_NS From ba5f531619d0eba24ec82298c382085b0d5f7c8d Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sun, 25 Apr 2021 00:44:51 -0400 Subject: [PATCH 12/24] add some basic tests for the "processors" command --- unittest/commands/test_simple_commands.cpp | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/unittest/commands/test_simple_commands.cpp b/unittest/commands/test_simple_commands.cpp index e8cebe98e2..ac8317b3da 100644 --- a/unittest/commands/test_simple_commands.cpp +++ b/unittest/commands/test_simple_commands.cpp @@ -14,6 +14,7 @@ #include "lammps.h" #include "citeme.h" +#include "comm.h" #include "force.h" #include "info.h" #include "input.h" @@ -25,6 +26,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include "../testing/core.h" +#include "../testing/utils.h" #include #include @@ -174,6 +176,38 @@ TEST_F(SimpleCommandsTest, Partition) ASSERT_THAT(text, StrEq("")); } +TEST_F(SimpleCommandsTest, Processors) +{ + // default setting is "*" for all dimensions + ASSERT_EQ(lmp->comm->user_procgrid[0], 0); + ASSERT_EQ(lmp->comm->user_procgrid[1], 0); + ASSERT_EQ(lmp->comm->user_procgrid[2], 0); + + BEGIN_HIDE_OUTPUT(); + command("processors 1 1 1"); + END_HIDE_OUTPUT(); + ASSERT_EQ(lmp->comm->user_procgrid[0], 1); + ASSERT_EQ(lmp->comm->user_procgrid[1], 1); + ASSERT_EQ(lmp->comm->user_procgrid[2], 1); + + BEGIN_HIDE_OUTPUT(); + command("processors * 1 *"); + END_HIDE_OUTPUT(); + ASSERT_EQ(lmp->comm->user_procgrid[0], 0); + ASSERT_EQ(lmp->comm->user_procgrid[1], 1); + ASSERT_EQ(lmp->comm->user_procgrid[2], 0); + + BEGIN_HIDE_OUTPUT(); + command("processors 0 0 0"); + END_HIDE_OUTPUT(); + ASSERT_EQ(lmp->comm->user_procgrid[0], 0); + ASSERT_EQ(lmp->comm->user_procgrid[1], 0); + ASSERT_EQ(lmp->comm->user_procgrid[2], 0); + + TEST_FAILURE(".*ERROR: Illegal processors command .*", command("processors -1 0 0");); + TEST_FAILURE(".*ERROR: Specified processors != physical processors.*", command("processors 100 100 100");); +} + TEST_F(SimpleCommandsTest, Quit) { BEGIN_HIDE_OUTPUT(); From b7088a14ae784fb1bd1b7c7171b3a2adee237286 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sun, 25 Apr 2021 00:45:10 -0400 Subject: [PATCH 13/24] use alternate way to compare strings --- unittest/cplusplus/test_lammps_class.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/unittest/cplusplus/test_lammps_class.cpp b/unittest/cplusplus/test_lammps_class.cpp index 6a0e6719cc..8f12bd247b 100644 --- a/unittest/cplusplus/test_lammps_class.cpp +++ b/unittest/cplusplus/test_lammps_class.cpp @@ -13,6 +13,8 @@ using ::testing::MatchesRegex; using ::testing::StartsWith; +using ::testing::StrEq; +using ::testing::StrNe; namespace LAMMPS_NS { // test fixture for regular tests @@ -91,13 +93,13 @@ TEST_F(LAMMPS_plain, InitMembers) EXPECT_NE(lmp->python, nullptr); EXPECT_EQ(lmp->citeme, nullptr); if (LAMMPS::has_git_info) { - EXPECT_STRNE(LAMMPS::git_commit, ""); - EXPECT_STRNE(LAMMPS::git_branch, ""); - EXPECT_STRNE(LAMMPS::git_descriptor, ""); + EXPECT_THAT(LAMMPS::git_commit, StrNe("")); + EXPECT_THAT(LAMMPS::git_branch, StrNe("")); + EXPECT_THAT(LAMMPS::git_descriptor, StrNe("")); } else { - EXPECT_STREQ(LAMMPS::git_commit, "(unknown)"); - EXPECT_STREQ(LAMMPS::git_branch, "(unknown)"); - EXPECT_STREQ(LAMMPS::git_descriptor, "(unknown)"); + EXPECT_THAT(LAMMPS::git_commit, StrEq("(unknown)")); + EXPECT_THAT(LAMMPS::git_branch, StrEq("(unknown)")); + EXPECT_THAT(LAMMPS::git_descriptor, StrEq("(unknown)")); } EXPECT_EQ(lmp->comm->nthreads, 1); } From ba4781bd82228ac5aa386785b000a972e4aa4d3b Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sun, 25 Apr 2021 01:14:57 -0400 Subject: [PATCH 14/24] restore old string matching as it works just as well (on my machine) --- unittest/cplusplus/test_lammps_class.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/unittest/cplusplus/test_lammps_class.cpp b/unittest/cplusplus/test_lammps_class.cpp index 8f12bd247b..6a0e6719cc 100644 --- a/unittest/cplusplus/test_lammps_class.cpp +++ b/unittest/cplusplus/test_lammps_class.cpp @@ -13,8 +13,6 @@ using ::testing::MatchesRegex; using ::testing::StartsWith; -using ::testing::StrEq; -using ::testing::StrNe; namespace LAMMPS_NS { // test fixture for regular tests @@ -93,13 +91,13 @@ TEST_F(LAMMPS_plain, InitMembers) EXPECT_NE(lmp->python, nullptr); EXPECT_EQ(lmp->citeme, nullptr); if (LAMMPS::has_git_info) { - EXPECT_THAT(LAMMPS::git_commit, StrNe("")); - EXPECT_THAT(LAMMPS::git_branch, StrNe("")); - EXPECT_THAT(LAMMPS::git_descriptor, StrNe("")); + EXPECT_STRNE(LAMMPS::git_commit, ""); + EXPECT_STRNE(LAMMPS::git_branch, ""); + EXPECT_STRNE(LAMMPS::git_descriptor, ""); } else { - EXPECT_THAT(LAMMPS::git_commit, StrEq("(unknown)")); - EXPECT_THAT(LAMMPS::git_branch, StrEq("(unknown)")); - EXPECT_THAT(LAMMPS::git_descriptor, StrEq("(unknown)")); + EXPECT_STREQ(LAMMPS::git_commit, "(unknown)"); + EXPECT_STREQ(LAMMPS::git_branch, "(unknown)"); + EXPECT_STREQ(LAMMPS::git_descriptor, "(unknown)"); } EXPECT_EQ(lmp->comm->nthreads, 1); } From beca3e5f0dfab1b00d33043424cc97b43ead011c Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sun, 25 Apr 2021 22:27:36 -0400 Subject: [PATCH 15/24] collect the full help message --- unittest/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittest/CMakeLists.txt b/unittest/CMakeLists.txt index 26db526b60..2d86fa2663 100644 --- a/unittest/CMakeLists.txt +++ b/unittest/CMakeLists.txt @@ -15,7 +15,7 @@ add_test(NAME HelpMessage COMMAND $ -h WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) set_tests_properties(HelpMessage PROPERTIES - ENVIRONMENT "TSAN_OPTIONS=ignore_noninstrumented_modules=1;PAGER=head" + ENVIRONMENT "TSAN_OPTIONS=ignore_noninstrumented_modules=1" PASS_REGULAR_EXPRESSION ".*Large-scale Atomic/Molecular Massively Parallel Simulator -.*Usage example:.*") # check if the compiled executable will error out on an invalid command line flag From 792966a957e8afbfba3ccde9682de19da8468972 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Mon, 26 Apr 2021 11:02:15 -0400 Subject: [PATCH 16/24] always describe the git version, even when using a git clone without history --- cmake/Modules/generate_lmpgitversion.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/Modules/generate_lmpgitversion.cmake b/cmake/Modules/generate_lmpgitversion.cmake index 4ff01c7501..b19716d74b 100644 --- a/cmake/Modules/generate_lmpgitversion.cmake +++ b/cmake/Modules/generate_lmpgitversion.cmake @@ -17,7 +17,7 @@ if(GIT_FOUND AND EXISTS ${LAMMPS_DIR}/.git) ERROR_QUIET WORKING_DIRECTORY ${LAMMPS_DIR} OUTPUT_STRIP_TRAILING_WHITESPACE) - execute_process(COMMAND ${GIT_EXECUTABLE} describe --dirty=-modified + execute_process(COMMAND ${GIT_EXECUTABLE} describe --dirty=-modified --always OUTPUT_VARIABLE temp_git_describe ERROR_QUIET WORKING_DIRECTORY ${LAMMPS_DIR} From 4fa5840f136236d1eb47dda73471b37d6d9b9f6a Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Mon, 26 Apr 2021 11:02:41 -0400 Subject: [PATCH 17/24] fix bug due to adding ArgInfo --- src/compute_chunk_atom.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/compute_chunk_atom.cpp b/src/compute_chunk_atom.cpp index 26967a1b12..16090eb0ec 100644 --- a/src/compute_chunk_atom.cpp +++ b/src/compute_chunk_atom.cpp @@ -155,6 +155,7 @@ ComputeChunkAtom::ComputeChunkAtom(LAMMPS *lmp, int narg, char **arg) : if ((which == ArgInfo::UNKNOWN) || (which == ArgInfo::NONE) || (argi.get_dim() > 1)) error->all(FLERR,"Illegal compute chunk/atom command"); + iarg = 4; } // optional args From ac60cfb0c32a0db720118ca09f158f6f943a6a97 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sat, 17 Apr 2021 15:41:45 -0400 Subject: [PATCH 18/24] add custom constructor for TextFileReader that uses an already opened file descriptor --- src/text_file_reader.cpp | 14 +++++++++++++- src/text_file_reader.h | 3 ++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/text_file_reader.cpp b/src/text_file_reader.cpp index b0d5bef53e..e16edab4eb 100644 --- a/src/text_file_reader.cpp +++ b/src/text_file_reader.cpp @@ -41,7 +41,7 @@ using namespace LAMMPS_NS; * \param filetype Description of file type for error messages */ TextFileReader::TextFileReader(const std::string &filename, const std::string &filetype) - : filename(filename), filetype(filetype), ignore_comments(true) + : filetype(filetype), ignore_comments(true) { fp = fopen(filename.c_str(), "r"); @@ -51,6 +51,18 @@ TextFileReader::TextFileReader(const std::string &filename, const std::string &f } } +/** + * \overload + * + * \param fp File descriptor of the already opened file + * \param filetype Description of file type for error messages */ + +TextFileReader::TextFileReader(FILE *fp, const std::string &filetype) + : filetype(filetype), fp(fp), ignore_comments(true) +{ + if (fp == nullptr) throw FileReaderException("Invalid file descriptor"); +} + /** Closes the file */ TextFileReader::~TextFileReader() { diff --git a/src/text_file_reader.h b/src/text_file_reader.h index 1b7ca73fed..bfd6e558ff 100644 --- a/src/text_file_reader.h +++ b/src/text_file_reader.h @@ -25,7 +25,6 @@ namespace LAMMPS_NS { class TextFileReader { - std::string filename; std::string filetype; static constexpr int MAXLINE = 1024; char line[MAXLINE]; @@ -35,6 +34,8 @@ namespace LAMMPS_NS bool ignore_comments; //!< Controls whether comments are ignored TextFileReader(const std::string &filename, const std::string &filetype); + TextFileReader(FILE *fp, const std::string &filetype); + ~TextFileReader(); void skip_line(); From 8af1530e29ba3a726ffaf164c921e98507ae7319 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Sun, 18 Apr 2021 04:04:40 -0400 Subject: [PATCH 19/24] throw EOF exception in TextFileReader::next_values() if next_line() doesn't do it --- src/text_file_reader.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/text_file_reader.cpp b/src/text_file_reader.cpp index e16edab4eb..61bd803aa8 100644 --- a/src/text_file_reader.cpp +++ b/src/text_file_reader.cpp @@ -175,5 +175,8 @@ void TextFileReader::next_dvector(double * list, int n) { * \return ValueTokenizer object for read in text */ ValueTokenizer TextFileReader::next_values(int nparams, const std::string &separators) { - return ValueTokenizer(next_line(nparams), separators); + char *ptr = next_line(nparams); + if (ptr == nullptr) + throw EOFException(fmt::format("Missing line in {} file!", filetype)); + return ValueTokenizer(line, separators); } From dbd7d454b9b1226e9510771bc1b58c57e147e1b7 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Mon, 26 Apr 2021 12:12:19 -0400 Subject: [PATCH 20/24] for consistent behavior we must not close the file pointer when it was passed as argument --- src/text_file_reader.cpp | 6 +++--- src/text_file_reader.h | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/text_file_reader.cpp b/src/text_file_reader.cpp index 61bd803aa8..7344c6d5c3 100644 --- a/src/text_file_reader.cpp +++ b/src/text_file_reader.cpp @@ -41,7 +41,7 @@ using namespace LAMMPS_NS; * \param filetype Description of file type for error messages */ TextFileReader::TextFileReader(const std::string &filename, const std::string &filetype) - : filetype(filetype), ignore_comments(true) + : filetype(filetype), closefp(true), ignore_comments(true) { fp = fopen(filename.c_str(), "r"); @@ -58,7 +58,7 @@ TextFileReader::TextFileReader(const std::string &filename, const std::string &f * \param filetype Description of file type for error messages */ TextFileReader::TextFileReader(FILE *fp, const std::string &filetype) - : filetype(filetype), fp(fp), ignore_comments(true) + : filetype(filetype), closefp(false), fp(fp), ignore_comments(true) { if (fp == nullptr) throw FileReaderException("Invalid file descriptor"); } @@ -66,7 +66,7 @@ TextFileReader::TextFileReader(FILE *fp, const std::string &filetype) /** Closes the file */ TextFileReader::~TextFileReader() { - fclose(fp); + if (closefp) fclose(fp); } /** Read the next line and ignore it */ diff --git a/src/text_file_reader.h b/src/text_file_reader.h index bfd6e558ff..327d57c059 100644 --- a/src/text_file_reader.h +++ b/src/text_file_reader.h @@ -26,6 +26,7 @@ namespace LAMMPS_NS { class TextFileReader { std::string filetype; + bool closefp; static constexpr int MAXLINE = 1024; char line[MAXLINE]; FILE *fp; From 2c6fe2d0b546b63e226e1f29d06808f342f941c4 Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Mon, 26 Apr 2021 12:12:45 -0400 Subject: [PATCH 21/24] add tests for the overloaded constructor using a file pointer --- unittest/formats/test_text_file_reader.cpp | 34 ++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/unittest/formats/test_text_file_reader.cpp b/unittest/formats/test_text_file_reader.cpp index e0bb2d42b5..3294881352 100644 --- a/unittest/formats/test_text_file_reader.cpp +++ b/unittest/formats/test_text_file_reader.cpp @@ -76,6 +76,40 @@ TEST_F(TextFileReaderTest, permissions) unlink("text_reader_noperms.file"); } +TEST_F(TextFileReaderTest, nofp) +{ + ASSERT_THROW({ TextFileReader reader(nullptr, "test"); }, + FileReaderException); +} + +TEST_F(TextFileReaderTest, usefp) +{ + test_files(); + FILE *fp = fopen("text_reader_two.file","r"); + ASSERT_NE(fp,nullptr); + + auto reader = new TextFileReader(fp, "test"); + auto line = reader->next_line(); + ASSERT_STREQ(line, "4 "); + line = reader->next_line(1); + ASSERT_STREQ(line, "4 0.5 "); + ASSERT_NO_THROW({ reader->skip_line(); }); + auto values = reader->next_values(1); + ASSERT_EQ(values.count(), 2); + ASSERT_EQ(values.next_int(), 3); + ASSERT_STREQ(values.next_string().c_str(), "1.5"); + ASSERT_NE(reader->next_line(), nullptr); + double data[20]; + ASSERT_THROW({ reader->next_dvector(data,20); }, FileReaderException); + ASSERT_THROW({ reader->skip_line(); }, EOFException); + ASSERT_EQ(reader->next_line(), nullptr); + delete reader; + + // check that we reached EOF and the destructor didn't close the file. + ASSERT_EQ(feof(fp),1); + ASSERT_EQ(fclose(fp),0); +} + TEST_F(TextFileReaderTest, comments) { test_files(); From 0eee2d013d962cfc182772b2192c607affc5c9dd Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Mon, 26 Apr 2021 12:15:03 -0400 Subject: [PATCH 22/24] add info to docs --- src/text_file_reader.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/text_file_reader.cpp b/src/text_file_reader.cpp index 7344c6d5c3..291d4348ea 100644 --- a/src/text_file_reader.cpp +++ b/src/text_file_reader.cpp @@ -54,6 +54,17 @@ TextFileReader::TextFileReader(const std::string &filename, const std::string &f /** * \overload * +\verbatim embed:rst + +This function is useful in combination with :cpp:func:`utils::open_potential`. + +.. note:: + + The FILE pointer is not closed in the destructor, but will be advanced + when reading from it. + +\endverbatim + * * \param fp File descriptor of the already opened file * \param filetype Description of file type for error messages */ From 462f27d661d924cab8c1a6b35645c94ee5f5d6d7 Mon Sep 17 00:00:00 2001 From: Richard Berger Date: Mon, 26 Apr 2021 14:28:13 -0400 Subject: [PATCH 23/24] Use copy-and-swap in Tokenizers Ensures that the classes behave consistently when copied, moved, copy assigned, and move assigned. --- src/tokenizer.cpp | 41 ++++++++++++++++++++ src/tokenizer.h | 10 +++-- unittest/utils/test_tokenizer.cpp | 64 +++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 4 deletions(-) diff --git a/src/tokenizer.cpp b/src/tokenizer.cpp index ea8ff2ce43..38cdfa73fb 100644 --- a/src/tokenizer.cpp +++ b/src/tokenizer.cpp @@ -68,6 +68,28 @@ Tokenizer::Tokenizer(Tokenizer && rhs) : reset(); } +Tokenizer& Tokenizer::operator=(const Tokenizer& other) +{ + Tokenizer tmp(other); + swap(tmp); + return *this; +} + +Tokenizer& Tokenizer::operator=(Tokenizer&& other) +{ + Tokenizer tmp(std::move(other)); + swap(tmp); + return *this; +} + +void Tokenizer::swap(Tokenizer& other) +{ + std::swap(text, other.text); + std::swap(separators, other.separators); + std::swap(start, other.start); + std::swap(ntokens, other.ntokens); +} + /*! Re-position the tokenizer state to the first word, * i.e. the first non-separator character */ void Tokenizer::reset() { @@ -181,6 +203,25 @@ ValueTokenizer::ValueTokenizer(const ValueTokenizer &rhs) : tokens(rhs.tokens) { ValueTokenizer::ValueTokenizer(ValueTokenizer &&rhs) : tokens(std::move(rhs.tokens)) { } +ValueTokenizer& ValueTokenizer::operator=(const ValueTokenizer& other) +{ + ValueTokenizer tmp(other); + swap(tmp); + return *this; +} + +ValueTokenizer& ValueTokenizer::operator=(ValueTokenizer&& other) +{ + ValueTokenizer tmp(std::move(other)); + swap(tmp); + return *this; +} + +void ValueTokenizer::swap(ValueTokenizer& other) +{ + std::swap(tokens, other.tokens); +} + /*! Indicate whether more tokens are available * * \return true if there are more tokens, false if not */ diff --git a/src/tokenizer.h b/src/tokenizer.h index a17ab13d04..8cc6d2d42b 100644 --- a/src/tokenizer.h +++ b/src/tokenizer.h @@ -37,8 +37,9 @@ public: Tokenizer(const std::string &str, const std::string &separators = TOKENIZER_DEFAULT_SEPARATORS); Tokenizer(Tokenizer &&); Tokenizer(const Tokenizer &); - Tokenizer& operator=(const Tokenizer&) = default; - Tokenizer& operator=(Tokenizer&&) = default; + Tokenizer& operator=(const Tokenizer&); + Tokenizer& operator=(Tokenizer&&); + void swap(Tokenizer &); void reset(); void skip(int n=1); @@ -93,8 +94,9 @@ public: ValueTokenizer(const std::string &str, const std::string &separators = TOKENIZER_DEFAULT_SEPARATORS); ValueTokenizer(const ValueTokenizer &); ValueTokenizer(ValueTokenizer &&); - ValueTokenizer& operator=(const ValueTokenizer&) = default; - ValueTokenizer& operator=(ValueTokenizer&&) = default; + ValueTokenizer& operator=(const ValueTokenizer&); + ValueTokenizer& operator=(ValueTokenizer&&); + void swap(ValueTokenizer &); std::string next_string(); tagint next_tagint(); diff --git a/unittest/utils/test_tokenizer.cpp b/unittest/utils/test_tokenizer.cpp index 5b20d24e7c..275a86a05f 100644 --- a/unittest/utils/test_tokenizer.cpp +++ b/unittest/utils/test_tokenizer.cpp @@ -101,6 +101,38 @@ TEST(Tokenizer, move_constructor) ASSERT_EQ(u.count(), 3); } +TEST(Tokenizer, copy_assignment) +{ + Tokenizer t(" test word ", " "); + Tokenizer u(" test2 word2 other2 ", " "); + ASSERT_THAT(t.next(), Eq("test")); + ASSERT_THAT(t.next(), Eq("word")); + ASSERT_EQ(t.count(), 2); + Tokenizer v = u; + u = t; + ASSERT_THAT(u.next(), Eq("test")); + ASSERT_THAT(u.next(), Eq("word")); + ASSERT_EQ(u.count(), 2); + + ASSERT_THAT(v.next(), Eq("test2")); + ASSERT_THAT(v.next(), Eq("word2")); + ASSERT_THAT(v.next(), Eq("other2")); + ASSERT_EQ(v.count(), 3); +} + +TEST(Tokenizer, move_assignment) +{ + Tokenizer t(" test word ", " "); + ASSERT_THAT(t.next(), Eq("test")); + ASSERT_THAT(t.next(), Eq("word")); + ASSERT_EQ(t.count(), 2); + t = Tokenizer("test new word ", " "); + ASSERT_THAT(t.next(), Eq("test")); + ASSERT_THAT(t.next(), Eq("new")); + ASSERT_THAT(t.next(), Eq("word")); + ASSERT_EQ(t.count(), 3); +} + TEST(Tokenizer, no_separator_path) { Tokenizer t("one", ":"); @@ -223,6 +255,38 @@ TEST(ValueTokenizer, move_constructor) ASSERT_EQ(u.count(), 3); } +TEST(ValueTokenizer, copy_assignment) +{ + ValueTokenizer t(" test word ", " "); + ValueTokenizer u(" test2 word2 other2 ", " "); + ASSERT_THAT(t.next_string(), Eq("test")); + ASSERT_THAT(t.next_string(), Eq("word")); + ASSERT_EQ(t.count(), 2); + ValueTokenizer v = u; + u = t; + ASSERT_THAT(u.next_string(), Eq("test")); + ASSERT_THAT(u.next_string(), Eq("word")); + ASSERT_EQ(u.count(), 2); + + ASSERT_THAT(v.next_string(), Eq("test2")); + ASSERT_THAT(v.next_string(), Eq("word2")); + ASSERT_THAT(v.next_string(), Eq("other2")); + ASSERT_EQ(v.count(), 3); +} + +TEST(ValueTokenizer, move_assignment) +{ + ValueTokenizer t(" test word ", " "); + ASSERT_THAT(t.next_string(), Eq("test")); + ASSERT_THAT(t.next_string(), Eq("word")); + ASSERT_EQ(t.count(), 2); + t = ValueTokenizer("test new word ", " "); + ASSERT_THAT(t.next_string(), Eq("test")); + ASSERT_THAT(t.next_string(), Eq("new")); + ASSERT_THAT(t.next_string(), Eq("word")); + ASSERT_EQ(t.count(), 3); +} + TEST(ValueTokenizer, bad_integer) { ValueTokenizer values("f10 f11 f12"); From 57a7bd7186df2aa8bddef3be5912d9653a7199bb Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Mon, 26 Apr 2021 20:16:55 -0400 Subject: [PATCH 24/24] adjust for changed CMake variable scope due to moving script code --- unittest/force-styles/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittest/force-styles/CMakeLists.txt b/unittest/force-styles/CMakeLists.txt index 60e83e7e16..2c0977397a 100644 --- a/unittest/force-styles/CMakeLists.txt +++ b/unittest/force-styles/CMakeLists.txt @@ -33,7 +33,7 @@ else() target_link_libraries(style_tests PUBLIC mpi_stubs) endif() # propagate sanitizer options to test tools -if (NOT ENABLE_SANITIZER STREQUAL "none") +if (ENABLE_SANITIZER AND (NOT ENABLE_SANITIZER STREQUAL "none")) if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.13) target_compile_options(style_tests PUBLIC -fsanitize=${ENABLE_SANITIZER}) target_link_options(style_tests PUBLIC -fsanitize=${ENABLE_SANITIZER})