Skip to content

Commit

Permalink
Normalize subfile path in H5 lib
Browse files Browse the repository at this point in the history
This has bothered me for a long time. A leading slash and
_no_ extension for subfile names was required before,
and failing to do this lead to rather useless error messages.
Now, the leading slash and the extension are handled
within the H5 lib, so it should be much easier to use.
  • Loading branch information
nilsvu committed Apr 23, 2024
1 parent 6645460 commit 78bc61d
Show file tree
Hide file tree
Showing 10 changed files with 20 additions and 43 deletions.
13 changes: 2 additions & 11 deletions src/IO/Exporter/Exporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ template <size_t Dim>
std::vector<std::vector<double>> interpolate_to_points(
const std::variant<std::vector<std::string>, std::string>&
volume_files_or_glob,
std::string subfile_name,
const std::string& subfile_name,
const std::variant<ObservationId, ObservationStep>& observation,
const std::vector<std::string>& tensor_components,
const std::array<std::vector<double>, Dim>& target_points,
Expand All @@ -173,15 +173,6 @@ std::vector<std::vector<double>> interpolate_to_points(
ERROR_NO_TRACE("No volume files found. Specify at least one volume file.");
}

// Normalize subfile name
if (subfile_name.size() > 4 &&
subfile_name.substr(subfile_name.size() - 4) == ".vol") {
subfile_name = subfile_name.substr(0, subfile_name.size() - 4);
}
if (subfile_name.front() != '/') {
subfile_name = '/' + subfile_name;
}

// Retrieve info from the first volume file
const h5::H5File<h5::AccessType::ReadOnly> first_h5file(filenames.front());
const auto& first_volfile = first_h5file.get<h5::VolumeData>(subfile_name);
Expand Down Expand Up @@ -278,7 +269,7 @@ std::vector<std::vector<double>> interpolate_to_points(
template std::vector<std::vector<double>> interpolate_to_points<DIM(data)>( \
const std::variant<std::vector<std::string>, std::string>& \
volume_files_or_glob, \
std::string subfile_name, \
const std::string& subfile_name, \
const std::variant<ObservationId, ObservationStep>& observation, \
const std::vector<std::string>& tensor_components, \
const std::array<std::vector<double>, DIM(data)>& target_points, \
Expand Down
2 changes: 1 addition & 1 deletion src/IO/Exporter/Exporter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ template <size_t Dim>
std::vector<std::vector<double>> interpolate_to_points(
const std::variant<std::vector<std::string>, std::string>&
volume_files_or_glob,
std::string subfile_name,
const std::string& subfile_name,
const std::variant<ObservationId, ObservationStep>& observation,
const std::vector<std::string>& tensor_components,
const std::array<std::vector<double>, Dim>& target_points,
Expand Down
13 changes: 11 additions & 2 deletions src/IO/H5/File.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ class H5File {

template <typename ObjectType>
std::tuple<bool, detail::OpenGroup, std::string> check_if_object_exists(
const std::string& path) const;
std::string path) const;

std::string file_name_;
// NOLINTNEXTLINE(spectre-mutable)
Expand Down Expand Up @@ -392,7 +392,16 @@ const ObjectType& H5File<Access_t>::convert_to_derived(
template <AccessType Access_t>
template <typename ObjectType>
std::tuple<bool, detail::OpenGroup, std::string>
H5File<Access_t>::check_if_object_exists(const std::string& path) const {
H5File<Access_t>::check_if_object_exists(std::string path) const {
// Normalize path to have a leading slash and the correct extension
if (path.front() != '/') {
path = '/' + path;
}
const size_t extension_length = ObjectType::extension().size();
if (path.size() > extension_length and
path.substr(path.size() - extension_length) == ObjectType::extension()) {
path = path.substr(0, path.size() - extension_length);
}
std::string name_only = "/";
if (path != "/") {
name_only = file_system::get_file_name(path);
Expand Down
5 changes: 0 additions & 5 deletions src/IO/H5/Python/CombineH5.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,6 @@ def combine_h5_vol_command(h5files, subfile_name, output, check_src):
rich.print(rich.columns.Columns(spectre_file.all_vol_files()))
return

if not subfile_name.startswith("/"):
subfile_name = "/" + subfile_name
if subfile_name.endswith(".vol"):
subfile_name = subfile_name[:-4]

if not output.endswith(".h5"):
output += ".h5"

Expand Down
5 changes: 0 additions & 5 deletions src/IO/H5/Python/ExtendConnectivityData.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,6 @@ def extend_connectivity_data_command(filename, subfile_name):
files, the combine-h5 executable must be run first. The extend-connectivity
command can then be run on the newly generated HDF5 file.
"""
# Ensure that the format of the subfile is correct
if subfile_name.endswith(".vol"):
subfile_name = subfile_name[:-4]
if subfile_name[0] != "/":
subfile_name = "/" + subfile_name
# Read the h5 file and extract the volume file from it
h5_file = spectre_h5.H5File(filename, "r+")
vol_file = h5_file.get_vol(subfile_name)
Expand Down
6 changes: 0 additions & 6 deletions src/Visualization/Python/OpenVolfiles.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,6 @@ def command(
rich.print(rich.columns.Columns(available_subfiles))
return

# Normalize subfile name
if subfile_name.endswith(".vol"):
subfile_name = subfile_name[:-4]
if not subfile_name.startswith("/"):
subfile_name = "/" + subfile_name

# Print available observations/times and exit
if list_times:
import rich.columns
Expand Down
5 changes: 0 additions & 5 deletions src/Visualization/Python/Render1D.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,6 @@ def render_1d_command(
rich.print(rich.columns.Columns(open_h5_files[0].all_vol_files()))
return

if subfile_name.endswith(".vol"):
subfile_name = subfile_name[:-4]
if not subfile_name.startswith("/"):
subfile_name = "/" + subfile_name

volfiles = [h5file.get_vol(subfile_name) for h5file in open_h5_files]
dim = volfiles[0].get_dimension()
assert (
Expand Down
5 changes: 0 additions & 5 deletions src/Visualization/Python/TransformVolumeData.py
Original file line number Diff line number Diff line change
Expand Up @@ -800,11 +800,6 @@ def shift_magnitude(
rich.print(rich.columns.Columns())
return

if subfile_name.endswith(".vol"):
subfile_name = subfile_name[:-4]
if not subfile_name.startswith("/"):
subfile_name = "/" + subfile_name

volfiles = [h5file.get_vol(subfile_name) for h5file in open_h5_files]

# Load kernels
Expand Down
4 changes: 3 additions & 1 deletion tests/Unit/IO/H5/Test_Dat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,9 @@ void test_dat_read() {
error_file.append(std::vector<double>{0.33, 0.66, 0.77, 0.90});
}
h5::H5File<h5::AccessType::ReadOnly> my_file(h5_file_name);
const auto& error_file = my_file.get<h5::Dat>("/L2_errors");
// No leading slash should also find the subfile, and a ".dat" extension as
// well
const auto& error_file = my_file.get<h5::Dat>("L2_errors.dat");
CHECK(error_file.subfile_path() == "/L2_errors");

// Check version info is correctly retrieved from Dat file
Expand Down
5 changes: 3 additions & 2 deletions tests/Unit/IO/H5/Test_VolumeData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,10 @@ void test() {
my_file.close_current_object();
}
// Open the read volume file and check that the observation id and values are
// correct.
// correct. No leading slash should also find the subfile, and a ".vol"
// extension as well.
const auto& volume_file =
my_file.get<h5::VolumeData>("/element_data", version_number);
my_file.get<h5::VolumeData>("element_data.vol", version_number);
CHECK(volume_file.subfile_path() == "/element_data");
const auto read_observation_ids = volume_file.list_observation_ids();
// The observation IDs should be sorted by their observation value
Expand Down

0 comments on commit 78bc61d

Please sign in to comment.