Skip to content

Commit

Permalink
bugfix: avoid dup ref path in relay io hdf5 error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
cyrush committed May 13, 2021
1 parent 9d5fb23 commit e5ceeed
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 13 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -33,6 +33,9 @@ and this project aspires to adhere to [Semantic Versioning](https://semver.org/s
- Fixed missing implementation of DataType::is_index_t
- Fixed issue with compiling t_h5z_zfp_smoke.cpp against an MPI-enabled HDF5.

#### Blueprint
- Fixed a bug that caused HDF5 reference paths to appear twice in Relay HDF5 Error messages.

#### Blueprint
- `conduit::relay::io::blueprint.read_mesh` now uses read only I/O handles.

Expand Down
23 changes: 13 additions & 10 deletions src/libs/relay/conduit_relay_io_hdf5.cpp
Expand Up @@ -38,17 +38,17 @@
//-----------------------------------------------------------------------------
#define CONDUIT_HDF5_ERROR( ref_path, msg ) \
{ \
CONDUIT_ERROR( "HDF5 Error (reference path: \"" << ref_path \
<< ref_path << "\") " << msg); \
CONDUIT_ERROR( "HDF5 Error (reference path: \"" << ref_path << "\") " \
<< msg); \
}

//-----------------------------------------------------------------------------
/// The CONDUIT_HDF5_WARN macro is used for warnings with ref paths.
//-----------------------------------------------------------------------------
#define CONDUIT_HDF5_WARN( ref_path, msg ) \
{ \
CONDUIT_WARN( "HDF5 Warning (reference path: \"" << ref_path \
<< ref_path << "\") " << msg); \
#define CONDUIT_HDF5_WARN( ref_path, msg ) \
{ \
CONDUIT_WARN( "HDF5 Warning (reference path: \"" << ref_path << "\") " \
<< msg); \
}

//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -1853,7 +1853,7 @@ setup_hdf5_group_atts_for_conduit_node(const Node &node,

if( has_list_attr && node.dtype().is_object() )
{
std::cout << " remove_conduit_hdf5_list_attribute " << std::endl;
// std::cout << " remove_conduit_hdf5_list_attribute " << std::endl;
remove_conduit_hdf5_list_attribute(hdf5_group_id,
ref_path);
}
Expand Down Expand Up @@ -2256,9 +2256,12 @@ h5l_iterate_traverse_op_func(hid_t hdf5_id,
<< " parent: " << hdf5_id
<< " path:" << hdf5_path) ;

std::string chld_ref_path = h5_od->ref_path +
std::string("/") +
std::string(hdf5_path);
std::string chld_ref_path = h5_od->ref_path;
if(chld_ref_path != std::string("/"))
{
chld_ref_path += std::string("/");
}
chld_ref_path += std::string(hdf5_path);

switch (h5_info_buf.type)
{
Expand Down
68 changes: 65 additions & 3 deletions src/tests/relay/t_relay_io_handle.cpp
Expand Up @@ -552,11 +552,11 @@ TEST(conduit_relay_io_handle, test_offset_and_stride)

h.read(n_read);
n_read.print();

n_check = n;
// expect no diff
EXPECT_FALSE(n_read.diff(n_check,info));

// strided read
n_read.reset();
opts.reset();
Expand Down Expand Up @@ -591,7 +591,7 @@ TEST(conduit_relay_io_handle, test_offset_and_stride)
n_read.reset();
h.read("data",n_read,opts);
n_read.print();

n_check.reset();
n_check = {0,1,2,3,4};
// expect no diff
Expand Down Expand Up @@ -772,3 +772,65 @@ TEST(conduit_relay_io_handle, test_empty_path_as_root)
EXPECT_FALSE(n.diff(n_read_2, info, 0.0));
EXPECT_FALSE(n.diff(n_read_2, info, 0.0));
}


//-----------------------------------------------------------------------------
TEST(conduit_relay_io_handle, test_ref_path_error_msg)
{
// check that ref path only appears in the error message string once

Node n_about;
io::about(n_about);

// skip test if hdf5 isn't enabled
if(n_about["protocols/hdf5"].as_string() != "enabled")
return;

std::string tfile_out = "tout_hdf5_io_handle_for_ref_path_error_msg.hdf5";
// remove files if they already exist
utils::remove_path_if_exists(tfile_out);

Node n, n_read, n_check, opts, info;
n["my/path/to/some/data"]= { 0,1,2,3,4,5,6,7,8,9};

io::IOHandle h;
h.open(tfile_out);
h.write(n);

h.read(n_read);
n_read.print();

n_check = n;
// expect no diff
EXPECT_FALSE(n_read.diff(n_check,info));

// huge offset
n_read.reset();
opts.reset();
opts["offset"] = 1000;
try
{
h.read(n_read,opts);
}
catch(conduit::Error &e)
{
std::string msg = e.message();
std::cout << "error message:"
<< msg << std::endl;
int count = 0;

std::string::size_type pos = 0;
std::string path = "my/path/to/some/data";

while ((pos = msg.find(path, pos )) != std::string::npos)
{
count++;
pos += path.length();
}

std::cout << "# of occurrences of path: " << count << std::endl;
// the path should only appear in the error message string once
EXPECT_EQ(count,1);
}

}
54 changes: 54 additions & 0 deletions src/tests/relay/t_relay_io_hdf5.cpp
Expand Up @@ -1873,3 +1873,57 @@ TEST(conduit_relay_io_hdf5, conduit_hdf5_compat_with_empty)

EXPECT_FALSE(n.diff(n_load,n_diff_info));
}

//-----------------------------------------------------------------------------
TEST(conduit_relay_io_hdf5, test_ref_path_error_msg)
{
// check that ref path only appears in the error message string once

Node n_about;
io::about(n_about);

// skip test if hdf5 isn't enabled
if(n_about["protocols/hdf5"].as_string() != "enabled")
return;

std::string tfile_out = "tout_hdf5_io_for_ref_path_error_msg.hdf5";
// remove files if they already exist
utils::remove_path_if_exists(tfile_out);

Node n, n_read, n_check, opts, info;
n["my/path/to/some/data"]= { 0,1,2,3,4,5,6,7,8,9};

io::save(n,tfile_out, "hdf5");

// bad offset
opts.reset();
opts["offset"] = 1000;

try
{
io::load(tfile_out,"hdf5",opts,n_read);
}
catch(conduit::Error &e)
{
std::string msg = e.message();
std::cout << "error message:"
<< msg << std::endl;
int count = 0;

std::string::size_type pos = 0;
std::string path = "my/path/to/some/data";

while ((pos = msg.find(path, pos )) != std::string::npos)
{
count++;
pos += path.length();
}

std::cout << "# of occurrences of path: " << count << std::endl;

// the path should only appear in the error message string once
EXPECT_EQ(count,1);
}

}

0 comments on commit e5ceeed

Please sign in to comment.