Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

improve error messages for incompatible hdf5 trees #534

Merged
merged 4 commits into from Apr 8, 2020
Merged

improve error messages for incompatible hdf5 trees #534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8fae91b
provide more context for hdf5 vs conduit incompat issues
cyrush Apr 7, 2020
d162be5
init some char buffers (valgrind ident)
cyrush Apr 7, 2020
afa1301
update change log
cyrush Apr 7, 2020
8fc8381
update valgrind supressions file
cyrush Apr 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -32,6 +32,10 @@ and this project aspires to adhere to [Semantic Versioning](https://semver.org/s
#### General
- Node::fetch_child and Schema::fetch_child are deprecated in favor of the more clearly named Node::fetch_existing and Schema::fetch_existing. fetch_child variants still exist, but will be removed in a future release.

#### Relay
- Provide more context when a Conduit Node cannot be written to a HDF5 file because it is incompatible with the existing HDF5 tree. Error messages now provide the full path and details about the incompatibility.


## [0.5.1] - Released 2020-01-18

### Added
Expand Down
57 changes: 55 additions & 2 deletions src/cmake/valgrind.supp
View file
Open in desktop
Expand Up @@ -115,12 +115,12 @@
fun:COMMENT_THIS_LINE_TO_DISABLE_LEAK_WARNING
}

#
######################################
# OSX leaks
#

{
Handle OSX specific Leak (possibly leaked)
mac os leak ignore 1
Memcheck:Leak
match-leak-kinds: possible
fun:malloc_zone_malloc
Expand All @@ -136,6 +136,59 @@
fun:_ZL22copySwiftV1MangledNamePKcb
fun:_ZL22copySwiftV1MangledNamePKcb
}
#
{
mac os leak ignore 2
Memcheck:Leak
match-leak-kinds: possible
fun:malloc_zone_calloc
...
fun:environ_init
fun:_os_object_init
}
#
{
mac os leak ignore 3
Memcheck:Leak
match-leak-kinds: possible
fun:malloc_zone_malloc
...
fun:_hasObjcContents
fun:__objc_personality_v0
}
#
{
mac os leak ignore 4
Memcheck:Leak
match-leak-kinds: possible
fun:malloc_zone_malloc
...
fun:_dyld_objc_notify_register
fun:environ_init
fun:_os_object_init
fun:libdispatch_init
}
#
{
mac os leak ignore 5
Memcheck:Leak
match-leak-kinds: possible
fun:malloc_zone_malloc
fun:_ZN10objc_class14setHasCustomRREb
fun:_ZL11addSubclassP10objc_classS0_
fun:_ZNSt3__112__hash_tableINS_17__hash_value_typeImPN23objc_references_support20ObjectAssociationMapEEENS_22__unordered_map_hasherImS5_NS2_20DisguisedPointerHashELb1EEENS_21__unordered_map_equalImS5_NS2_21DisguisedPointerEqualELb1EEENS2_13ObjcAllocatorIS5_EEE8__rehashEm
fun:_ZL18updateCustomRR_AWZP10objc_classP8method_t
fun:_ZL9addMethodP10objc_classP13objc_selectorPFvvEPKcb
fun:_ZL17realizeAllClassesv
fun:_objc_flush_caches
fun:_objc_flush_caches
fun:_objc_flush_caches
fun:_objc_flush_caches
fun:_read_images
}
#####################################
# end macos leaks
#####################################

#
# Non-python specific leaks
Expand Down
2 changes: 1 addition & 1 deletion src/libs/conduit/conduit_utils.cpp
View file
Open in desktop
Expand Up @@ -951,7 +951,7 @@ base64_decode(const void *src,
std::string
float64_to_string(float64 value)
{
char buffer[64];
char buffer[64] = {0};
snprintf(buffer,64,"%.15g",value);

std::string res(buffer);
Expand Down
132 changes: 104 additions & 28 deletions src/libs/relay/conduit_relay_io_hdf5.cpp
View file
Open in desktop
Expand Up @@ -393,21 +393,26 @@ void hdf5_ref_path_with_filename(hid_t hdf5_id,
// helpers for checking if compatible
//-----------------------------------------------------------------------------

//-----------------------------------------------------------------------------
// if incompatible, incompat_details contains human readable details
// about why
//-----------------------------------------------------------------------------
bool check_if_conduit_leaf_is_compatible_with_hdf5_obj(const DataType &dtype,
const std::string &ref_path,
hid_t hdf5_id);
hid_t hdf5_id,
std::string &incompat_details);

//-----------------------------------------------------------------------------
bool check_if_conduit_object_is_compatible_with_hdf5_tree(const Node &node,
const std::string &ref_path,
hid_t hdf5_id);
hid_t hdf5_id,
std::string &incompat_details);

//-----------------------------------------------------------------------------
bool check_if_conduit_node_is_compatible_with_hdf5_tree(const Node &node,
const std::string &ref_path,
hid_t hdf5_id);

hid_t hdf5_id,
std::string &incompat_details);

//-----------------------------------------------------------------------------
// helpers for writing
Expand Down Expand Up @@ -833,7 +838,8 @@ hdf5_ref_path_with_filename(hid_t hdf5_id,
bool
check_if_conduit_leaf_is_compatible_with_hdf5_obj(const DataType &dtype,
const std::string &ref_path,
hid_t hdf5_id)
hid_t hdf5_id,
std::string &incompat_details)
{
bool res = true;
H5O_info_t h5_obj_info;
Expand All @@ -851,6 +857,15 @@ check_if_conduit_leaf_is_compatible_with_hdf5_obj(const DataType &dtype,
if( H5Sget_simple_extent_type(h5_test_dspace) == H5S_NULL &&
!dtype.is_empty())
{
std::ostringstream oss;
oss << "Conduit Node (leaf) at path '" << ref_path << "'"
<< " is not compatible with given HDF5 Dataset at path"
<< " '" << ref_path << "'"
<< "\nHDF5 dataset has a H5S_NULL Dataspace which"
<< " only compatible with an empty Conduit Node";

incompat_details = oss.str();

res = false;
}
else
Expand Down Expand Up @@ -878,12 +893,32 @@ check_if_conduit_leaf_is_compatible_with_hdf5_obj(const DataType &dtype,
// note: both hdf5 and conduit dtypes include null term in string size
(dtype.number_of_elements() != (index_t)H5Tget_size(h5_test_dtype) ) )
{
res = false;
std::ostringstream oss;
oss << "Conduit Node (string leaf) at path '" << ref_path << "'"
<< " is not compatible with given HDF5 Dataset at path"
<< " '" << ref_path << "'"
<< "\nConduit leaf String Node length ("
<< dtype.number_of_elements() << ")"
<< " != HDF5 Dataset size (" << H5Tget_size(h5_test_dtype) << ")";

incompat_details = oss.str();

res = false;
}
else if( ! ( (H5Tequal(h5_dtype, h5_test_dtype) > 0) &&
(dtype.number_of_elements() == h5_test_num_ele) ) )
{
res = false;
std::ostringstream oss;
oss << "Conduit Node (leaf) at path '" << ref_path << "'"
<< " is not compatible with given HDF5 Dataset at path"
<< " '" << ref_path << "'"
<< "\nConduit leaf Node number of elements ("
<< dtype.number_of_elements() << ")"
<< " != HDF5 Dataset size (" << H5Tget_size(h5_test_dtype) << ")";

incompat_details = oss.str();

res = false;
}

CONDUIT_CHECK_HDF5_ERROR_WITH_FILE_AND_REF_PATH(H5Tclose(h5_test_dtype),
Expand All @@ -903,15 +938,23 @@ check_if_conduit_leaf_is_compatible_with_hdf5_obj(const DataType &dtype,
else
{
// bad id, or not a dataset
std::ostringstream oss;
oss << "Conduit Node (leaf) at path '" << ref_path << "'"
<< " is not compatible with given HDF5 Dataset at path"
<< "'" << ref_path << "'"
<< "\nConduit leaf vs HDF5 Dataset: Bad HDF5 Leaf ID"
<< " or HDF5 ID is not a HDF5 Group";

incompat_details = oss.str();
res = false;
}

if(res == false)
{
CONDUIT_INFO("leaf in Conduit Node at path " << ref_path <<
" is not compatible with given HDF5 tree at path "
<< ref_path);
}
//
// if(res == false)
// {
// CONDUIT_INFO("leaf in Conduit Node at path " << ref_path <<
// " is not compatible with given HDF5 tree at path "
// << ref_path);
// }

return res;
}
Expand All @@ -920,9 +963,11 @@ check_if_conduit_leaf_is_compatible_with_hdf5_obj(const DataType &dtype,
bool
check_if_conduit_object_is_compatible_with_hdf5_tree(const Node &node,
const std::string &ref_path,
hid_t hdf5_id)
hid_t hdf5_id,
std::string &incompat_details)
{
bool res = true;

// make sure we have a group ...

H5O_info_t h5_obj_info;
Expand Down Expand Up @@ -953,7 +998,8 @@ check_if_conduit_object_is_compatible_with_hdf5_tree(const Node &node,
// compatible with the conduit node
res = check_if_conduit_node_is_compatible_with_hdf5_tree(child,
chld_ref_path,
h5_child_obj);
h5_child_obj,
incompat_details);

CONDUIT_CHECK_HDF5_ERROR_WITH_FILE_AND_REF_PATH(H5Oclose(h5_child_obj),
hdf5_id,
Expand All @@ -966,9 +1012,15 @@ check_if_conduit_object_is_compatible_with_hdf5_tree(const Node &node,
}
else // bad id or not a group
{
CONDUIT_INFO("object in Conduit Node at path " << ref_path <<
" is not compatible with given HDF5 tree at path "
<< ref_path );
std::ostringstream oss;
oss << "Conduit Node (object) at path '" << ref_path << "'"
<< " is not compatible with given HDF5 tree at path"
<< "'" << ref_path << "'"
<< "\nConduit Object vs HDF5 Group: Bad HDF5 Group ID "
<< "or HDF5 ID is not a HDF5 Group";

incompat_details = oss.str();

res = false;
}

Expand All @@ -980,29 +1032,46 @@ check_if_conduit_object_is_compatible_with_hdf5_tree(const Node &node,
bool
check_if_conduit_node_is_compatible_with_hdf5_tree(const Node &node,
const std::string &ref_path,
hid_t hdf5_id)
hid_t hdf5_id,
std::string &incompat_details)
{
bool res = true;

DataType dt = node.dtype();
// check for leaf or group
if(dt.is_number() || dt.is_string())
{
res = check_if_conduit_leaf_is_compatible_with_hdf5_obj(dt,
ref_path,
hdf5_id);
hdf5_id,
incompat_details);
}
else if(dt.is_object())
{
res = check_if_conduit_object_is_compatible_with_hdf5_tree(node,
ref_path,
hdf5_id);
hdf5_id,
incompat_details);
}
else // not supported
{
std::ostringstream oss;
oss << "Conduit Node at path '" << ref_path << "'"
<< " has an unsupported dtype (" << dt.name() << ")"
<< " for HDF5 i/o and cannot be written to HDF5 path"
<< " '" << ref_path << "'";

if(dt.is_list()) // let them know there is hope for lists in the future
{
// we don't support lists yet, but provide helpful
// info when passed a list
oss << "\nA future conduit release will add"
<< " list read/write support for HDF5.";
}
incompat_details = oss.str();
res = false;
}

return res;
}

Expand Down Expand Up @@ -2262,10 +2331,12 @@ hdf5_write(const Node &node,
n.set_external(const_cast<Node&>(node));
}

std::string incompat_details;
// check compat
if(check_if_conduit_node_is_compatible_with_hdf5_tree(n,
"",
hdf5_id))
hdf5_id,
incompat_details))
{
// write if we are compat
write_conduit_node_to_hdf5_tree(n,"",hdf5_id);
Expand All @@ -2280,7 +2351,8 @@ hdf5_write(const Node &node,
CONDUIT_ERROR("Failed to write node to "
<< "\"" << hdf5_error_ref_path << "\", "
<< "existing HDF5 tree is "
<< "incompatible with the Conduit Node.");
<< "incompatible with the Conduit Node."
<< "\nDetails:\n" << incompat_details);
}

// restore hdf5 error stack
Expand All @@ -2297,10 +2369,13 @@ hdf5_write(const Node &node,
// of check_if_conduit_node_is_compatible_with_hdf5_tree
HDF5ErrorStackSupressor supress_hdf5_errors;

std::string incompat_details;

// check compat
if(check_if_conduit_node_is_compatible_with_hdf5_tree(node,
"",
hdf5_id))
hdf5_id,
incompat_details))
{
// write if we are compat
write_conduit_node_to_hdf5_tree(node,
Expand All @@ -2315,7 +2390,8 @@ hdf5_write(const Node &node,
CONDUIT_ERROR("Failed to write node to "
<< "\"" << hdf5_fname << "\", "
<< "existing HDF5 tree is "
<< "incompatible with the Conduit Node.");
<< "incompatible with the Conduit Node."
<< " Details: " << incompat_details);
}
// restore hdf5 error stack
}
Expand Down
2 changes: 1 addition & 1 deletion src/tests/blueprint/t_blueprint_mpi_relay.cpp
View file
Open in desktop
Expand Up @@ -90,7 +90,7 @@ void mesh_blueprint_save(const Node &data,
int par_size = mpi::size(MPI_COMM_WORLD);

// setup the directory
char fmt_buff[64];
char fmt_buff[64] = {0};
int cycle = data["state/cycle"].to_int32();
snprintf(fmt_buff, sizeof(fmt_buff), "%06d",cycle);

Expand Down
25 changes: 25 additions & 0 deletions src/tests/relay/t_relay_io_hdf5.cpp
View file
Open in desktop
Expand Up @@ -1344,4 +1344,29 @@ TEST(conduit_relay_io_hdf5, conduit_hdf5_write_read_string_compress)

}

//-----------------------------------------------------------------------------
TEST(conduit_relay_io_hdf5, conduit_hdf5_list_incompat_error_message)
{
std::string tout_std = "tout_hdf5_w_list_error.hdf5";

Node n;
n.append() = "42";
n.append() = "42";
n.append() = "42";
n.append() = "42";

bool excpt_occured = false;
try
{
io::save(n,tout_std, "hdf5");
}
catch(Error &e)
{
excpt_occured = true;
CONDUIT_INFO(e.message());
}

EXPECT_TRUE(excpt_occured);
}