From c81610a3e8dc3becc87cd77691ad504171e388b4 Mon Sep 17 00:00:00 2001 From: "Ryan S. Elliott" Date: Tue, 16 Mar 2021 14:39:24 -0500 Subject: [PATCH 1/5] Check for installed OpenKIM models to be returned by kim query command --- src/KIM/kim_query.cpp | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/KIM/kim_query.cpp b/src/KIM/kim_query.cpp index cf3e6f51cf..8e9e7fc63f 100644 --- a/src/KIM/kim_query.cpp +++ b/src/KIM/kim_query.cpp @@ -62,6 +62,10 @@ #include "fix_store_kim.h" #include "info.h" #include "input.h" +extern "C" { +#include "KIM_Collections.h" +#include "KIM_CollectionItemType.h" +} #include "modify.h" #include "utils.h" #include "variable.h" @@ -185,6 +189,35 @@ void KimQuery::command(int narg, char **arg) input->write_echo("#=== BEGIN kim-query ==================================" "=======\n"); + // trim list of models to those that are installed on the system + if (query_function == "get_available_models") { + ValueTokenizer vals(value, ","); + std::string available; + std::string missing; + + KIM_Collections * col; + if (KIM_Collections_Create(&col)) { + delete [] value; + error->all(FLERR, fmt::format("Unable to create KIM_Collections object")); + } + + while (vals.has_next()) { + auto svalue = vals.next_string(); + KIM_CollectionItemType typ; + if (KIM_Collections_GetItemType(col, svalue.c_str(), &typ)) + missing += fmt::format("{} ", svalue); + else + available += fmt::format("{} ", svalue); + } + KIM_Collections_Destroy(&col); + + input->write_echo(fmt::format("# Installed OpenKIM models: {}\n", available)); + input->write_echo(fmt::format("# Missing OpenKIM models: {}\n", missing)); + // replace results with available + strcpy(value, available.c_str()); // available guaranteed to fit + }; + + ValueTokenizer values(value, ","); if (format_arg == "split") { int counter = 1; From 0e8f64251e1a211eeb243cb75e2afae38bbbe227 Mon Sep 17 00:00:00 2001 From: Yaser Afshar Date: Tue, 16 Mar 2021 16:23:54 -0500 Subject: [PATCH 2/5] Correct the token to avoid missing model names Correct the missing & available strings to match with the rest of query Add error message when there is no OpenKIM model installed on the system Correct the header file inlcusion order --- src/KIM/kim_query.cpp | 44 +++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/src/KIM/kim_query.cpp b/src/KIM/kim_query.cpp index 8e9e7fc63f..b344972689 100644 --- a/src/KIM/kim_query.cpp +++ b/src/KIM/kim_query.cpp @@ -62,10 +62,6 @@ #include "fix_store_kim.h" #include "info.h" #include "input.h" -extern "C" { -#include "KIM_Collections.h" -#include "KIM_CollectionItemType.h" -} #include "modify.h" #include "utils.h" #include "variable.h" @@ -82,6 +78,11 @@ extern "C" { #include #endif +extern "C" { +#include "KIM_Collections.h" +#include "KIM_CollectionItemType.h" +} + using namespace LAMMPS_NS; #if defined(LMP_KIM_CURL) @@ -191,33 +192,44 @@ void KimQuery::command(int narg, char **arg) "=======\n"); // trim list of models to those that are installed on the system if (query_function == "get_available_models") { - ValueTokenizer vals(value, ","); + ValueTokenizer vals(value, ", \""); std::string available; std::string missing; - KIM_Collections * col; - if (KIM_Collections_Create(&col)) { + KIM_Collections * collections; + KIM_CollectionItemType typ; + + if (KIM_Collections_Create(&collections)) { delete [] value; - error->all(FLERR, fmt::format("Unable to create KIM_Collections object")); + error->all(FLERR, + fmt::format("Unable to access KIM Collections to find Model")); } + auto logID = fmt::format("{}_Collections", comm->me); + KIM_Collections_SetLogID(collections, logID.c_str()); + while (vals.has_next()) { auto svalue = vals.next_string(); - KIM_CollectionItemType typ; - if (KIM_Collections_GetItemType(col, svalue.c_str(), &typ)) - missing += fmt::format("{} ", svalue); + if (KIM_Collections_GetItemType(collections, svalue.c_str(), &typ)) + missing += fmt::format("{}, ", svalue); else - available += fmt::format("{} ", svalue); + available += fmt::format("{}, ", svalue); } - KIM_Collections_Destroy(&col); + KIM_Collections_Destroy(&collections); + + input->write_echo( + fmt::format("# Missing OpenKIM models: {}\n\n", missing)); + + if (available.empty()) + error->all(FLERR, + fmt::format("There is no OpenKIM model installed on the system")); + input->write_echo( + fmt::format("# Installed OpenKIM models: {}\n\n", available)); - input->write_echo(fmt::format("# Installed OpenKIM models: {}\n", available)); - input->write_echo(fmt::format("# Missing OpenKIM models: {}\n", missing)); // replace results with available strcpy(value, available.c_str()); // available guaranteed to fit }; - ValueTokenizer values(value, ","); if (format_arg == "split") { int counter = 1; From a6773bad5d2de71b63e013a9f0b297bb393c7bd2 Mon Sep 17 00:00:00 2001 From: Yaser Afshar Date: Tue, 16 Mar 2021 19:40:49 -0500 Subject: [PATCH 3/5] remove unnecessary echo --- src/KIM/kim_query.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/KIM/kim_query.cpp b/src/KIM/kim_query.cpp index b344972689..cc4ece4406 100644 --- a/src/KIM/kim_query.cpp +++ b/src/KIM/kim_query.cpp @@ -223,8 +223,6 @@ void KimQuery::command(int narg, char **arg) if (available.empty()) error->all(FLERR, fmt::format("There is no OpenKIM model installed on the system")); - input->write_echo( - fmt::format("# Installed OpenKIM models: {}\n\n", available)); // replace results with available strcpy(value, available.c_str()); // available guaranteed to fit From 199595c5105fbd605ada5616aefa21f78f7df0df Mon Sep 17 00:00:00 2001 From: "Ryan S. Elliott" Date: Wed, 17 Mar 2021 10:54:55 -0500 Subject: [PATCH 4/5] Small adjustment to log message --- src/KIM/kim_query.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/KIM/kim_query.cpp b/src/KIM/kim_query.cpp index cc4ece4406..789509682d 100644 --- a/src/KIM/kim_query.cpp +++ b/src/KIM/kim_query.cpp @@ -222,7 +222,8 @@ void KimQuery::command(int narg, char **arg) if (available.empty()) error->all(FLERR, - fmt::format("There is no OpenKIM model installed on the system")); + fmt::format( + "There are no matching OpenKIM models installed on the system")); // replace results with available strcpy(value, available.c_str()); // available guaranteed to fit From d5a1591cd11f87c4641106083fcc9ad5fdff506a Mon Sep 17 00:00:00 2001 From: Axel Kohlmeyer Date: Thu, 18 Mar 2021 12:12:30 -0400 Subject: [PATCH 5/5] simplify code - use Tokenizer instead of ValueTokenizer since no value conversions are needed - don't use fmt::format() when no string formatting is needed --- src/KIM/kim_query.cpp | 71 +++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 37 deletions(-) diff --git a/src/KIM/kim_query.cpp b/src/KIM/kim_query.cpp index 789509682d..74b0179302 100644 --- a/src/KIM/kim_query.cpp +++ b/src/KIM/kim_query.cpp @@ -119,38 +119,36 @@ void KimQuery::command(int narg, char **arg) } ++arg; --narg; - // The “list” is the default setting + // The "list" is the default setting // the result is returned as a space-separated list of values in a variable } else format_arg = "list"; - std::string query_function{arg[1]}; + std::string query_function(arg[1]); if (query_function == "split" || query_function == "list" || - query_function == "index") { - auto msg = fmt::format("Illegal 'kim query' command.\nThe '{}' keyword " - "can not be used after '{}'", query_function, format_arg); - error->all(FLERR, msg); - } + query_function == "index") + error->all(FLERR, fmt::format("Illegal 'kim query' command.\nThe '{}' " + "keyword can not be used after '{}'", + query_function, format_arg)); std::string model_name; // check the query_args format (a series of keyword=value pairs) for (int i = 2; i < narg; ++i) { - if (!utils::strmatch(arg[i], "[=][\\[].*[\\]]")) { - auto msg = fmt::format("Illegal query format.\nInput argument " - "of `{}` to 'kim query' is wrong. The query format is the " - "keyword=[value], where value is always an array of one or " - "more comma-separated items", arg[i]); - error->all(FLERR, msg); - } + if (!utils::strmatch(arg[i], "[=][\\[].*[\\]]")) + error->all(FLERR, fmt::format("Illegal query format.\nInput argument " + "of `{}` to 'kim query' is wrong. The " + "query format is the keyword=[value], " + "where value is always an array of one or " + "more comma-separated items", arg[i])); } if (query_function != "get_available_models") { for (int i = 2; i < narg; ++i) { // check if the model is specified as an argument if (utils::strmatch(arg[i], "^model=")) { - ValueTokenizer values(arg[i], "=[]"); - std::string key = values.next_string(); - model_name = values.next_string(); + Tokenizer values(arg[i], "=[]"); + values.skip(1); + model_name = values.next(); break; } } @@ -161,12 +159,12 @@ void KimQuery::command(int narg, char **arg) if (ifix >= 0) { FixStoreKIM *fix_store = (FixStoreKIM *) modify->fix[ifix]; char *model_name_c = (char *) fix_store->getptr("model_name"); - model_name = fmt::format("{}", model_name_c); + model_name = model_name_c; } else { - auto msg = fmt::format("Illegal query format.\nMust use 'kim init' " - "before 'kim query' or must provide the model name after query " - "function with the format of 'model=[model_name]'"); - error->all(FLERR, msg); + error->all(FLERR, "Illegal query format.\nMust use 'kim init' " + "before 'kim query' or must provide the model name " + "after query function with the format of " + "'model=[model_name]'"); } } } @@ -185,31 +183,30 @@ void KimQuery::command(int narg, char **arg) error->all(FLERR, msg); } else if (strcmp(value, "EMPTY") == 0) { delete [] value; - error->all(FLERR, fmt::format("OpenKIM query returned no results")); + error->all(FLERR, "OpenKIM query returned no results"); } input->write_echo("#=== BEGIN kim-query ==================================" "=======\n"); // trim list of models to those that are installed on the system if (query_function == "get_available_models") { - ValueTokenizer vals(value, ", \""); + Tokenizer vals(value, ", \""); std::string available; std::string missing; - KIM_Collections * collections; + KIM_Collections *collections; KIM_CollectionItemType typ; if (KIM_Collections_Create(&collections)) { delete [] value; - error->all(FLERR, - fmt::format("Unable to access KIM Collections to find Model")); + error->all(FLERR, "Unable to access KIM Collections to find Model"); } auto logID = fmt::format("{}_Collections", comm->me); KIM_Collections_SetLogID(collections, logID.c_str()); while (vals.has_next()) { - auto svalue = vals.next_string(); + auto svalue = vals.next(); if (KIM_Collections_GetItemType(collections, svalue.c_str(), &typ)) missing += fmt::format("{}, ", svalue); else @@ -220,34 +217,34 @@ void KimQuery::command(int narg, char **arg) input->write_echo( fmt::format("# Missing OpenKIM models: {}\n\n", missing)); - if (available.empty()) - error->all(FLERR, - fmt::format( - "There are no matching OpenKIM models installed on the system")); + if (available.empty()) { + delete [] value; + error->all(FLERR,"There are no matching OpenKIM models installed on the system"); + } // replace results with available strcpy(value, available.c_str()); // available guaranteed to fit }; - ValueTokenizer values(value, ","); + Tokenizer values(value, ","); if (format_arg == "split") { int counter = 1; while (values.has_next()) { - auto svalue = values.next_string(); + auto svalue = values.next(); auto setcmd = fmt::format("{}_{} string {}", var_name, counter++, svalue); input->variable->set(setcmd); input->write_echo(fmt::format("variable {}\n", setcmd)); } } else { std::string setcmd; - auto svalue = utils::trim(values.next_string()); + auto svalue = utils::trim(values.next()); if (format_arg == "list") { setcmd = fmt::format("{} string \"", var_name); setcmd += (svalue.front() == '"' && svalue.back() == '"') ? fmt::format("{}", svalue.substr(1, svalue.size() - 2)) : fmt::format("{}", svalue); while (values.has_next()) { - svalue = utils::trim(values.next_string()); + svalue = utils::trim(values.next()); setcmd += (svalue.front() == '"' && svalue.back() == '"') ? fmt::format(" {}", svalue.substr(1, svalue.size() - 2)) : fmt::format(" {}", svalue); @@ -257,7 +254,7 @@ void KimQuery::command(int narg, char **arg) // format_arg == "index" setcmd = fmt::format("{} index {}", var_name, svalue); while (values.has_next()) { - svalue = values.next_string(); + svalue = values.next(); setcmd += fmt::format(" {}", svalue); } }