Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib/include/resdata/rd_kw.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,10 @@ inline rd_kw_ptr make_rd_kw(const char *header, int size,
}

inline std::string rd_kw_iget_stripped_string(const rd_kw_type *kw, int index) {
return rd::strip_spaces(
static_cast<const char *>(rd_kw_iget_ptr(kw, index)));
const char *raw = static_cast<const char *>(rd_kw_iget_ptr(kw, index));
const size_t width = rd_type_get_sizeof_iotype(rd_kw_get_data_type(kw));
size_t len = 0;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be nice with some unit-tests for this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a few now

while (len < width && raw[len] != '\0')
len++;
return rd::strip_spaces(std::string(raw, len));
}
Comment thread
MagnusSletten marked this conversation as resolved.
5 changes: 5 additions & 0 deletions lib/resdata/rd_file_view.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ void rd_file_view_index_fload_kw(const rd_file_view_type *rd_file_view,
char *io_buffer) {
auto file_kw = rd_file_view_iget_named_file_kw(rd_file_view, kw, index);

if (!file_kw)
throw std::invalid_argument(std::string("Keyword '") + kw + "' index " +
std::to_string(index) +
" not found in file view");

if (rd_file_view->fortio->assert_stream_open()) {
offset_type offset = file_kw->get_offset();
rd_data_type data_type = file_kw->get_data_type();
Expand Down
29 changes: 28 additions & 1 deletion lib/resdata/rd_smspec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,9 @@ static void rd_smspec_load_restart(rd_smspec_type *rd_smspec,
if (!rd_file_has_kw(header, RESTART_KW))
return;
const rd_kw_type *restart_kw = rd_file_iget_named_kw(header, RESTART_KW, 0);
if (restart_kw == nullptr)
throw std::invalid_argument(
"RESTART keyword lookup failed despite keyword presence");
int num_blocks = rd_kw_get_size(restart_kw);
num_blocks = (num_blocks < 0) ? 0 : num_blocks;
auto tmp_base = rd::checked_calloc<char>(8 * num_blocks + 1);
Expand Down Expand Up @@ -871,17 +874,34 @@ static bool rd_smspec_fread_header(rd_smspec_type *rd_smspec,

int params_index;
rd_smspec->num_regions = 0;
rd_smspec->params_size = rd_kw_get_size(keywords);

if (wells == NULL)
throw std::invalid_argument(
"Could not locate WGNAMES/NAMES keyword in header");
if (keywords == NULL)
throw std::invalid_argument(
"Could not locate KEYWORDS keyword in header");
if (startdat == NULL)
throw std::invalid_argument(
"Could not locate STARTDAT keyword in header");
if (units == NULL)
throw std::invalid_argument(
"Could not locate UNITS keyword in header");
if (dimens == NULL)
throw std::invalid_argument(
"Could not locate DIMENS keyword in header");

rd_smspec->params_size = rd_kw_get_size(keywords);

if (rd_file_has_kw(header.get(), NUMS_KW))
nums = rd_file_iget_named_kw(header.get(), NUMS_KW, 0);

if (rd_file_has_kw(header.get(), INTEHEAD_KW)) {
const rd_kw_type *intehead =
rd_file_iget_named_kw(header.get(), INTEHEAD_KW, 0);
if (intehead == NULL)
throw std::invalid_argument(
Comment thread
eivindjahren marked this conversation as resolved.
"INTEHEAD keyword lookup failed despite keyword presence");
rd_smspec->unit_system = (ert_rd_unit_enum)rd_kw_iget_int(
intehead, INTEHEAD_SMSPEC_UNIT_INDEX);
/*
Expand All @@ -903,12 +923,16 @@ static bool rd_smspec_fread_header(rd_smspec_type *rd_smspec,
numlx = rd_file_iget_named_kw(header.get(), NUMLX_KW, 0);
numly = rd_file_iget_named_kw(header.get(), NUMLY_KW, 0);
numlz = rd_file_iget_named_kw(header.get(), NUMLZ_KW, 0);

rd_smspec->has_lgr = true;
} else
rd_smspec->has_lgr = false;

{
int *date = rd_kw_get_int_ptr(startdat);
if (date == NULL)
throw std::invalid_argument(
"STARTDAT keyword has no integer data payload");
int year = date[STARTDAT_YEAR_INDEX];
int month = date[STARTDAT_MONTH_INDEX];
int day = date[STARTDAT_DAY_INDEX];
Expand Down Expand Up @@ -1007,6 +1031,9 @@ rd_smspec_type *rd_smspec_fread_alloc(const std::string &header_file,
rd_smspec_get_var_node(rd_smspec->misc_var_index, "TIME");
if (time_node) {
const char *time_unit = time_node->get_unit();
if (time_unit == nullptr)
throw std::invalid_argument(
"TIME variable is missing a unit string");
rd_smspec->time_index = time_node->get_params_index();

if (util_string_equal(time_unit, "DAYS"))
Expand Down
24 changes: 20 additions & 4 deletions lib/resdata/rd_unsmry_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

#include <resdata/rd_kw_magic.hpp>
#include <resdata/rd_file.hpp>
#include <resdata/rd_file_view.hpp>
#include <resdata/rd_file_kw.hpp>
#include <resdata/rd_type.hpp>

#include "detail/resdata/rd_unsmry_loader.hpp"

Expand Down Expand Up @@ -36,6 +39,20 @@ unsmry_loader::unsmry_loader(const rd_smspec_type *smspec,
rd_smspec_get_date_year_index(smspec)}};
rd_file_view_type *file_view = rd_file_get_global_view(file.get());
int length = rd_file_view_get_num_named_kw(file_view, PARAMS_KW);

if (length > 0) {
const rd_kw_type *params_kw =
rd_file_view_iget_named_kw(file_view, PARAMS_KW, 0);
if (params_kw == nullptr)
throw std::invalid_argument(
"Malformed summary file: missing PARAMS keyword entry");

const rd_data_type params_data_type = rd_kw_get_data_type(params_kw);
if (!rd_type_is_float(params_data_type))
throw std::invalid_argument(
"Malformed summary file: PARAMS keyword is not float");
}

this->file = file.release();
this->file_view = file_view;
this->m_length = length;
Expand All @@ -53,13 +70,12 @@ std::vector<double> unsmry_loader::get_vector(int pos) const {

std::vector<double> data(this->length());
auto index_map = make_int_vector(1, pos);
char buffer[4];
float value;

for (int index = 0; index < this->length(); index++) {
rd_file_view_index_fload_kw(file_view, PARAMS_KW, index,
index_map.get(), buffer);
float *data_value = (float *)buffer;
data[index] = *data_value;
index_map.get(), (char *)&value);
data[index] = value;
}

if (rd_file_view_flags_set(file_view, RD_FILE_CLOSE_STREAM))
Expand Down
61 changes: 55 additions & 6 deletions lib/resdata/smspec_node.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <cstring>
#include <stddef.h>
#include <cstdint>
#include <cmath>
#include <ctime>

Expand All @@ -8,6 +9,7 @@
#include <set>
#include <array>
#include <memory>
#include <limits>
#include <stdexcept>
#include <algorithm>
#include <vector>
Expand Down Expand Up @@ -570,12 +572,59 @@ void smspec_node::set_num(const int *grid_dims, int num_) {
this->num = num_;
if ((var_type == RD_SMSPEC_COMPLETION_VAR) ||
(var_type == RD_SMSPEC_BLOCK_VAR)) {
int global_index = this->num - 1;
this->ijk[2] = global_index / (grid_dims[0] * grid_dims[1]);
global_index -= this->ijk[2] * (grid_dims[0] * grid_dims[1]);
this->ijk[1] = global_index / grid_dims[0];
global_index -= this->ijk[1] * grid_dims[0];
this->ijk[0] = global_index;
if (grid_dims == nullptr)
throw std::invalid_argument(
"Grid dimensions are required for block and completion "
"summary variables");

if (grid_dims[0] <= 0 || grid_dims[1] <= 0 || grid_dims[2] <= 0)
throw std::invalid_argument(rd::util::string_format(
"Invalid grid dimensions for summary variable: nx=%d, ny=%d, "
"nz=%d",
grid_dims[0], grid_dims[1], grid_dims[2]));

const int64_t nx = grid_dims[0];
const int64_t ny = grid_dims[1];
const int64_t nz = grid_dims[2];

if (nx > std::numeric_limits<int64_t>::max() / ny)
throw std::invalid_argument(rd::util::string_format(
"Grid dimensions overflow for summary variable: nx=%d, ny=%d, "
"nz=%d",
grid_dims[0], grid_dims[1], grid_dims[2]));

const int64_t plane_size = nx * ny;
if (plane_size <= 0)
throw std::invalid_argument(rd::util::string_format(
"Invalid grid dimensions for summary variable: nx=%d, ny=%d, "
"nz=%d",
grid_dims[0], grid_dims[1], grid_dims[2]));

if (plane_size > std::numeric_limits<int64_t>::max() / nz)
throw std::invalid_argument(rd::util::string_format(
"Grid dimensions overflow for summary variable: nx=%d, ny=%d, "
"nz=%d",
grid_dims[0], grid_dims[1], grid_dims[2]));

const int64_t grid_size = plane_size * nz;
if (grid_size <= 0)
throw std::invalid_argument(rd::util::string_format(
"Invalid grid dimensions for summary variable: nx=%d, ny=%d, "
"nz=%d",
grid_dims[0], grid_dims[1], grid_dims[2]));

int64_t global_index = static_cast<int64_t>(this->num) - 1;
if (global_index < 0 || global_index >= grid_size)
throw std::invalid_argument(rd::util::string_format(
"Invalid NUMS value for summary variable: nums=%d, expected "
"1..%lld",
this->num, static_cast<long long>(grid_size)));

this->ijk[2] = static_cast<int>(global_index / plane_size);
global_index -= static_cast<int64_t>(this->ijk[2]) * plane_size;
this->ijk[1] = static_cast<int>(global_index / nx);
global_index -= static_cast<int64_t>(this->ijk[1]) * nx;
this->ijk[0] = static_cast<int>(global_index);

this->ijk[0] += 1;
this->ijk[1] += 1;
Expand Down
31 changes: 31 additions & 0 deletions lib/tests/test_rd_kw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,37 @@ TEST_CASE("rd_kw_iset_string_ptr validates type and length", "[rd_kw]") {
}
}

TEST_CASE("rd_kw_iget_stripped_string handles width edge cases", "[rd_kw]") {
SECTION("RD_CHAR values can fill the full field width") {
auto char_kw = make_rd_kw("KW", 2, RD_CHAR);

rd_kw_iset_char_ptr(char_kw.get(), 0, "FOPRTEST");
rd_kw_iset_char_ptr(char_kw.get(), 1, "BPR");

REQUIRE(rd_kw_iget_stripped_string(char_kw.get(), 0) == "FOPRTEST");
REQUIRE(rd_kw_iget_stripped_string(char_kw.get(), 1) == "BPR");
}

SECTION("RD_STRING values can fill the declared field width") {
auto string_kw = make_rd_kw("KW", 1, RD_STRING(12));

rd_kw_iset_string_ptr(string_kw.get(), 0, "0123456789AB");

REQUIRE(rd_kw_iget_stripped_string(string_kw.get(), 0) ==
"0123456789AB");
}

SECTION("embedded NUL stops the extracted string before field width") {
auto string_kw = make_rd_kw("KW", 1, RD_STRING(12));
char *raw = static_cast<char *>(rd_kw_iget_ptr(string_kw.get(), 0));

std::memcpy(raw, "ABCD\0EFGHIJK", 12);
raw[12] = '\0';

REQUIRE(rd_kw_iget_stripped_string(string_kw.get(), 0) == "ABCD");
}
}

TEST_CASE("scalar_set/scale/shift validate the type", "[rd_kw]") {
auto float_kw = make_rd_kw("KW", 3, RD_FLOAT);
REQUIRE_THROWS_WITH(rd_kw_scalar_set_int(float_kw.get(), 1),
Expand Down
96 changes: 96 additions & 0 deletions tests/rd_tests/test_rd_sum.py
Original file line number Diff line number Diff line change
Expand Up @@ -1524,3 +1524,99 @@ def test_get_general_var_index_with_unknown_key_raises():

with pytest.raises(IndexError, match="Invalid lookup summary object"):
summary.get_general_var_index("NO_SUCH_KEY")


@pytest.mark.usefixtures("use_tmpdir")
def test_smspec_keyword_without_nul_terminator_is_read_correctly():
smspec_kws = [
("INTEHEAD", np.array([1, 100], dtype=np.int32)),
("RESTART ", [" "] * 9),
("DIMENS ", np.array([2, 10, 10, 10, 0, 0], dtype=np.int32)),
("KEYWORDS", ["TIME ", "FOPRTEST"]),
("WGNAMES ", [":+:+:+:+", ":+:+:+:+"]),
("NUMS ", np.array([-32676, 0], dtype=np.int32)),
("UNITS ", ["DAYS ", "SM3 "]),
("STARTDAT", np.array([1, 1, 2000, 0, 0, 0], dtype=np.int32)),
]
resfo.write("TEST.SMSPEC", smspec_kws)

unsmry_kws = [
("SEQHDR ", np.array([1], dtype=np.int32)),
("MINISTEP", np.array([0], dtype=np.int32)),
("PARAMS ", np.array([1.0, 100.0], dtype=np.float32)),
]
resfo.write("TEST.UNSMRY", unsmry_kws)

summary = Summary("TEST")
assert summary.has_key("FOPRTEST")


@pytest.mark.usefixtures("use_tmpdir")
def test_smspec_missing_required_keyword_raises():
for missing_kw in ("WGNAMES ", "KEYWORDS", "UNITS ", "DIMENS "):
smspec_kws = [
("INTEHEAD", np.array([1, 100], dtype=np.int32)),
("RESTART ", [" "] * 9),
("DIMENS ", np.array([2, 10, 10, 10, 0, 0], dtype=np.int32)),
("KEYWORDS", ["TIME ", "FOPR "]),
("WGNAMES ", [":+:+:+:+", ":+:+:+:+"]),
("NUMS ", np.array([-32676, 0], dtype=np.int32)),
("UNITS ", ["DAYS ", "SM3 "]),
("STARTDAT", np.array([1, 1, 2000, 0, 0, 0], dtype=np.int32)),
]
smspec_kws = [(k, v) for k, v in smspec_kws if k != missing_kw]
resfo.write("TEST.SMSPEC", smspec_kws)
resfo.write(
"TEST.UNSMRY",
[
("SEQHDR ", np.array([1], dtype=np.int32)),
("MINISTEP", np.array([0], dtype=np.int32)),
("PARAMS ", np.array([1.0, 100.0], dtype=np.float32)),
],
)

with pytest.raises((IOError, ValueError)):
Summary("TEST")


@pytest.mark.usefixtures("use_tmpdir")
def test_smspec_startdat_with_empty_payload_raises():
smspec_kws = [
("INTEHEAD", np.array([1, 100], dtype=np.int32)),
("RESTART ", [" "] * 9),
("DIMENS ", np.array([2, 10, 10, 10, 0, 0], dtype=np.int32)),
("KEYWORDS", ["TIME ", "FOPR "]),
("WGNAMES ", [":+:+:+:+", ":+:+:+:+"]),
("NUMS ", np.array([-32676, 0], dtype=np.int32)),
("UNITS ", ["DAYS ", "SM3 "]),
("STARTDAT", ["NOTANINT"]),
]
resfo.write("TEST.SMSPEC", smspec_kws)
resfo.write(
"TEST.UNSMRY",
[
("SEQHDR ", np.array([1], dtype=np.int32)),
("MINISTEP", np.array([0], dtype=np.int32)),
("PARAMS ", np.array([1.0, 100.0], dtype=np.float32)),
],
)

with pytest.raises((IOError, ValueError)):
Summary("TEST")


@pytest.mark.usefixtures("use_tmpdir")
def test_unsmry_params_non_float_raises():
create_summary(summary_keys=("FOPR",), times=(0.0, 1.0, 2.0))

kws = list(resfo.read("TEST.UNSMRY"))
patched = []
for name, value in kws:
if name.strip() == "PARAMS":
patched.append((name, value.astype(np.int32)))
else:
patched.append((name, value))
resfo.write("TEST.UNSMRY", patched)

with pytest.raises((IOError, ValueError)):
Summary("TEST")
Loading