Skip to content

Commit

Permalink
Replace old GENERAL_OBSERVATION error message
Browse files Browse the repository at this point in the history
Replaced to dynamically report keywords for which files cannot be
located, topped off with an extensive unit-test. The method
conf_instance_has_path_error() is neither used not defined anywhere so
declaration is removed from conf.hpp
  • Loading branch information
BjarneHerland committed Nov 26, 2021
1 parent 1fa5533 commit a5794d6
Show file tree
Hide file tree
Showing 5 changed files with 231 additions and 24 deletions.
51 changes: 46 additions & 5 deletions libres/lib/config/conf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <vector>
#include <string>
#include <filesystem>
#include <iostream>

#include <assert.h>
#include <string.h>
Expand Down Expand Up @@ -1037,8 +1038,24 @@ conf_instance_validate_sub_instances(const conf_instance_type *conf_instance) {
return ok;
}

bool conf_instance_get_path_error(const conf_instance_type *conf_instance) {
bool path_errors = false;
/**
* This function resolves paths for each file-keyword in the given
* configuration. It returns a set of keywords with corresponding
* path for each non-existing path.
*
* The return type is std::set because this is a simple and efficient
* way to avoid duplicates in the end-result: Note that the method
* recurses and combines results in the last part of the code.
*
* Elements in the set are the keyword concatenated with "=>" and its
* resolved path. For example
*
* "OBS_FILE=>/var/tmp/obs_path/obs.txt"
*
*/
static std::set<std::string>
get_path_errors(const conf_instance_type *conf_instance) {
std::set<std::string> path_errors;
{
int num_items = hash_get_size(conf_instance->items);
char **item_keys = hash_alloc_keylist(conf_instance->items);
Expand All @@ -1050,7 +1067,8 @@ bool conf_instance_get_path_error(const conf_instance_type *conf_instance) {
conf_item->conf_item_spec;
if (conf_item_spec->dt == DT_FILE) {
if (!fs::exists(conf_item->value))
path_errors = true;
path_errors.insert(std::string(conf_item_spec->name) +
"=>" + std::string(conf_item->value));
}
}
util_free_stringlist(item_keys, num_items);
Expand All @@ -1067,8 +1085,8 @@ bool conf_instance_get_path_error(const conf_instance_type *conf_instance) {
const conf_instance_type *sub_conf_instance =
(const conf_instance_type *)hash_get(
conf_instance->sub_instances, sub_instances_key);
if (conf_instance_get_path_error(sub_conf_instance))
path_errors = true;
std::set<std::string> sub = get_path_errors(sub_conf_instance);
path_errors.insert(sub.begin(), sub.end());
}

util_free_stringlist(sub_instance_keys, num_sub_instances);
Expand All @@ -1077,6 +1095,29 @@ bool conf_instance_get_path_error(const conf_instance_type *conf_instance) {
return path_errors;
}

/**
* This method returns a single C-style string (zero-terminated char *)
* describing keywords and paths to which these resolve, and which does
* not exist. For example
*
* "OBS_FILE=>/var/tmp/obs_path/obs.txt\nINDEX_FILE=>/var/notexisting/file.txt"
*
* Note newlines - this string is intended for printing.
*/
char *conf_instance_get_path_error(const conf_instance_type *conf_instance) {
std::set<std::string> errors = get_path_errors(conf_instance);

if (errors.size() == 0)
return NULL;

std::string retval;
for (std::string s : errors) {
retval.append(s);
retval.append("\n");
}
return strdup(retval.data());
}

bool conf_instance_validate(const conf_instance_type *conf_instance) {
return conf_instance && conf_instance_has_required_items(conf_instance) &&
conf_instance_has_valid_mutexes(conf_instance) &&
Expand Down
25 changes: 11 additions & 14 deletions libres/lib/enkf/enkf_obs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -659,18 +659,14 @@ static void handle_general_observation(enkf_obs_type *enkf_obs,
stringlist_free(block_obs_keys);
}

static void enkf_obs_reinterpret_DT_FILE(const char *config_file) {
static void enkf_obs_reinterpret_DT_FILE(const char *errors) {
// clang-format off
fprintf(stderr, "**********************************************************************\n");
fprintf(stderr, "* In ert version 2.3 we have changed how filepaths are interpreted *\n");
fprintf(stderr, "* in the observation file. When using the keywords OBS_FILE, *\n");
fprintf(stderr, "* ERROR_COVAR and INDEX_FILE in the category GENERAL_OBSERVATION *\n");
fprintf(stderr, "* the filenames will be interpreted relative to the main observation *\n");
fprintf(stderr, "* file. *\n");
fprintf(stderr, "* *\n");
fprintf(stderr, "* Please update the OBS_FILE, ERROR_COVAR and INDEX_FILE keywords *\n");
fprintf(stderr, "* by removing the path to the main observation file. *\n");
fprintf(stderr, "**********************************************************************\n");
fprintf(stderr, "*****************************************\n");
fprintf(stderr, "The following keywords in your configuration did not resolve to a valid path: \n");
fprintf(stderr, "\n");
fprintf(stderr, "%s\n", errors);
fprintf(stderr, "\n");
fprintf(stderr, "*****************************************\n");
// clang-format on
}

Expand All @@ -693,9 +689,10 @@ void enkf_obs_load(enkf_obs_type *enkf_obs, const char *config_file,
conf_instance_type *enkf_conf = conf_instance_alloc_from_file(
enkf_conf_class, "enkf_conf", config_file);

if (conf_instance_get_path_error(enkf_conf)) {
enkf_obs_reinterpret_DT_FILE(config_file);
exit(1);
const char *errors = conf_instance_get_path_error(enkf_conf);
if (errors) {
enkf_obs_reinterpret_DT_FILE(errors);
exit(1); // No need to free errors...
}

if (!conf_instance_validate(enkf_conf))
Expand Down
5 changes: 1 addition & 4 deletions libres/lib/include/ert/config/conf.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@
#ifdef __cplusplus
extern "C" {
#endif

#include <stdbool.h>

#include <ert/util/stringlist.hpp>
Expand Down Expand Up @@ -247,14 +246,12 @@ time_t
conf_instance_get_item_value_time_t(const conf_instance_type *conf_instance,
const char *item_name);

bool conf_instance_get_path_error(const conf_instance_type *conf_instance);
char *conf_instance_get_path_error(const conf_instance_type *conf_instance);

/** V A L I D A T O R S */

bool conf_instance_validate(const conf_instance_type *conf_instance);

bool conf_instance_has_path_error(const conf_instance_type *conf_instance);

/** A L L O C F R O M F I L E */

conf_instance_type *
Expand Down
4 changes: 3 additions & 1 deletion libres/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ include(Catch)
add_executable(ert_test_suite tmpdir.cpp
analysis/modules/test_ies_enkf_main.cpp
analysis/test_enkf_linalg.cpp
res_util/test_matrix.cpp)
res_util/test_matrix.cpp
enkf/enkf_obs_paths_detailed.cpp
)
target_link_libraries(ert_test_suite res Catch2::Catch2WithMain fmt::fmt)

catch_discover_tests(ert_test_suite)
170 changes: 170 additions & 0 deletions libres/tests/enkf/enkf_obs_paths_detailed.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
#include <stdlib.h>
#include <string>
#include <fstream>

#include <catch2/catch.hpp>

#include <ert/enkf/enkf_obs.hpp>
#include "../tmpdir.hpp"

/*
* Write conf-file with given keywords, then parse it.
* Returns pointer to the parsed configuration
*/
conf_instance_type *write_conf(std::vector<std::string> keyword_lines) {
std::string buf("GENERAL_OBSERVATION WPR_DIFF_1 {\n");
buf += " DATA = SNAKE_OIL_WPR_DIFF;\n";
buf += " INDEX_LIST = 400,800,1200,1800;\n";
buf += " RESTART = 199;\n";
for (auto s : keyword_lines)
buf += s + "\n";
buf += "};\n";

std::ofstream stream("obs_path/conf.txt", std::ostream::out);
stream.write(buf.data(), buf.size());
stream.close(); // close before reading in next step!

conf_class_type *enkf_conf_class = enkf_obs_get_obs_conf_class();
return conf_instance_alloc_from_file(enkf_conf_class, "enkf_conf",
"obs_path/conf.txt");
}

/*
* Just a convenient wrapper from char* to std::string
*/
std::string get_path_error(conf_instance_type *enkf_conf) {
char *errors = conf_instance_get_path_error(enkf_conf);
if (errors) {
std::string s(errors);
free(errors);
return s;
} else
return std::string("");
}

/*
* Syntactic sugar for checking if a string contains another string
* The memmber-function ::contains() is introduced in C++23
*/
bool contains(std::string errors, std::string kw) {
return errors.find(kw) != std::string::npos;
}

/**
* Syntactic sugar for making sure a file exists ("touching" it)
*
* Note that std::ofstream is a proper RAII object which means
* neither flush nor close is required.
*/
void touch_file(std::string name) {
std::ofstream stream(name.data(), std::ostream::out);
}

TEST_CASE("Test parsing keywords in configuration", "[unittest]") {
GIVEN("A workearea") {
WITH_TMPDIR;
std::filesystem::create_directory("obs_path");

GIVEN("A conf file with no relevant keywords") {
conf_instance_type *enkf_conf = write_conf({});
THEN("There are no errors") {
std::string errors = get_path_error(enkf_conf);
REQUIRE(!contains(errors, "OBS_FILE"));
REQUIRE(!contains(errors, "ERROR_COVAR"));
REQUIRE(!contains(errors, "INDEX_FILE"));
}
conf_instance_free(enkf_conf);
}

GIVEN("A conf file with OBS_FILE keyword in it") {
conf_instance_type *enkf_conf =
write_conf({"OBS_FILE = obs.txt;"});
WHEN("There is no OBS_FILE present") {
THEN("Error refers to missing file") {
std::string errors = get_path_error(enkf_conf);
REQUIRE(contains(
errors, "OBS_FILE=>" +
std::filesystem::current_path().native() +
"/obs_path/obs.txt"));
REQUIRE(!contains(errors, "ERROR_COVAR"));
REQUIRE(!contains(errors, "INDEX_FILE"));
}
}
WHEN("The obs file is present") {
touch_file("obs_path/obs.txt");
THEN("There are no errors") {
std::string errors = get_path_error(enkf_conf);
REQUIRE(!contains(errors, "OBS_FILE"));
REQUIRE(!contains(errors, "ERROR_COVAR"));
REQUIRE(!contains(errors, "INDEX_FILE"));
}
}
conf_instance_free(enkf_conf);
}

GIVEN("A conf file with OBS_FILE in subdir obs_path") {
conf_instance_type *enkf_conf =
write_conf({"OBS_FILE = obs_path/obs.txt;"});

WHEN("The OBS_FILE is in the wrong location") {
touch_file("obs_path/obs.txt");
THEN("Error refers to missing file") {
std::string errors = get_path_error(enkf_conf);
REQUIRE(contains(
errors, "OBS_FILE=>" +
std::filesystem::current_path().native() +
"/obs_path/obs_path/obs.txt"));
REQUIRE(!contains(errors, "ERROR_COVAR"));
REQUIRE(!contains(errors, "INDEX_FILE"));
}
}
WHEN("The OBS_FILE is in the subdirectory") {
util_make_path("obs_path/obs_path");
touch_file("obs_path/obs_path/obs.txt");
THEN("There are no errors") {
std::string errors = get_path_error(enkf_conf);
REQUIRE(!contains(errors, "OBS_FILE"));
REQUIRE(!contains(errors, "ERROR_COVAR"));
REQUIRE(!contains(errors, "INDEX_FILE"));
}
}
conf_instance_free(enkf_conf);
}

GIVEN(
"A conf file with OBS_FILE, ERROR_COVAR and INDEX_FILE keywords") {
conf_instance_type *enkf_conf =
write_conf({"OBS_FILE = obs.txt;"
"ERROR_COVAR = covar.txt;"
"INDEX_FILE = index.txt;"});

WHEN("Only OBS_FILE exists") {
touch_file("obs_path/obs.txt");
THEN("Error refer to missing covar and index file") {
std::string errors = get_path_error(enkf_conf);
REQUIRE(!contains(errors, "OBS_FILE"));
REQUIRE(contains(
errors, "ERROR_COVAR=>" +
std::filesystem::current_path().native() +
"/obs_path/covar.txt"));
REQUIRE(contains(
errors, "INDEX_FILE=>" +
std::filesystem::current_path().native() +
"/obs_path/index.txt"));
}
}
WHEN("covar, obs, and index files exist") {
touch_file("obs_path/obs.txt");
touch_file("obs_path/covar.txt");
touch_file("obs_path/index.txt");
THEN("There are no errors") {
std::string errors = get_path_error(enkf_conf);
REQUIRE(!contains(errors, "OBS_FILE"));
REQUIRE(!contains(errors, "ERROR_COVAR"));
REQUIRE(!contains(errors, "INDEX_FILE"));
}
}
conf_instance_free(enkf_conf);
}
}
}

0 comments on commit a5794d6

Please sign in to comment.