Skip to content

Commit

Permalink
Fix corner cases of combine_selection. (#1038)
Browse files Browse the repository at this point in the history
  • Loading branch information
@1uc
1uc authored Aug 20, 2024
1 parent e9492c1 commit a6e17bd
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 20 deletions.
29 changes: 20 additions & 9 deletions include/highfive/bits/H5Slice_traits.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,25 @@ class HyperSlab {
DataSpace combine_selections(const DataSpace& left_space,
Op op,
const DataSpace& right_space) const {
return detail::make_data_space(
H5Scombine_select(left_space.getId(), convert(op), right_space.getId()));
assert(op == Op::Or);

auto left_type = detail::h5s_get_select_type(left_space.getId());
auto right_type = detail::h5s_get_select_type(right_space.getId());

// Since HDF5 doesn't allow `combine_selections` with a None
// selection, we need to avoid the issue:
if (left_type == H5S_SEL_NONE) {
return right_space;
} else if (right_type == H5S_SEL_NONE) {
return left_space;
} else if (left_type == H5S_SEL_ALL) {
return left_space;
} else if (right_type == H5S_SEL_ALL) {
return right_space;
} else {
return detail::make_data_space(
detail::h5s_combine_select(left_space.getId(), convert(op), right_space.getId()));
}
}

/// Reduce a sequence of `Op::Or` efficiently.
Expand Down Expand Up @@ -294,13 +311,7 @@ class HyperSlab {

if (n_ors > 1) {
auto right_space = reduce_streak(space_, begin, begin + n_ors, Op::Or);
// Since HDF5 doesn't allow `combine_selections` with a None
// selection, we need to avoid the issue:
if (detail::h5s_get_select_type(space.getId()) == H5S_SEL_NONE) {
space = right_space;
} else {
space = combine_selections(space, Op::Or, right_space);
}
space = combine_selections(space, Op::Or, right_space);
i += n_ors - 1;
} else if (selects[i].op == Op::None) {
detail::h5s_select_none(space.getId());
Expand Down
102 changes: 91 additions & 11 deletions tests/unit/test_high_five_selection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,17 @@ TEMPLATE_LIST_TEST_CASE("irregularHyperSlabSelectionWrite", "[template]", std::t
irregularHyperSlabSelectionWriteTest<TestType>();
}

void check_selected(const std::vector<int>& selected,
const std::vector<std::array<size_t, 2>>& indices,
const std::vector<std::vector<int>>& x) {
REQUIRE(selected.size() == indices.size());
for (size_t k = 0; k < selected.size(); ++k) {
size_t i = indices[k][0];
size_t j = indices[k][1];
REQUIRE(selected[k] == x[i][j]);
}
}

TEST_CASE("select_multiple_ors", "[hyperslab]") {
size_t n = 100, m = 20;
size_t nsel = 30;
Expand All @@ -558,12 +569,7 @@ TEST_CASE("select_multiple_ors", "[hyperslab]") {

SECTION("Pure Or Chain") {
auto selected = dset.select(hyperslab).read<std::vector<int>>();
REQUIRE(selected.size() == indices.size());
for (size_t k = 0; k < selected.size(); ++k) {
size_t i = indices[k][0];
size_t j = indices[k][1];
REQUIRE(selected[k] == x[i][j]);
}
check_selected(selected, indices, x);
}

SECTION("Or Chain And Slab") {
Expand All @@ -583,11 +589,85 @@ TEST_CASE("select_multiple_ors", "[hyperslab]") {
hyperslab &= RegularHyperSlab(offsets, counts);

auto selected = dset.select(hyperslab).read<std::vector<int>>();
REQUIRE(selected.size() == selected_indices.size());
for (size_t k = 0; k < selected.size(); ++k) {
size_t i = selected_indices[k][0];
size_t j = selected_indices[k][1];
REQUIRE(selected[k] == x[i][j]);
check_selected(selected, selected_indices, x);
}
}

TEST_CASE("select_multiple_ors_edge_cases", "[hyperslab]") {
size_t n = 100, m = 20;

auto x = testing::DataGenerator<std::vector<std::vector<int>>>::create({n, m});
auto file = File("select_multiple_ors_edge_cases.h5", File::Truncate);
auto dset = file.createDataSet("x", x);

std::vector<std::array<size_t, 2>> indices;
for (size_t i = 0; i < n; ++i) {
for (size_t j = 0; j < m; ++j) {
indices.push_back({i, j});
}
}

auto space = DataSpace({n, m});
SECTION("complete |= ") {
auto hyperslab = HyperSlab(RegularHyperSlab({0, 0}, {n, m}));

// Select everything; and then redundantly some parts again.
hyperslab &= RegularHyperSlab({0, 0}, {n, m});
hyperslab |= RegularHyperSlab({0, 0}, {n, m / 2});
hyperslab |= RegularHyperSlab({3, 0}, {1, 3});
hyperslab |= RegularHyperSlab({6, 0}, {1, 3});
hyperslab.apply(space);

auto selected = dset.select(hyperslab).read<std::vector<int>>();
check_selected(selected, indices, x);
}

SECTION("complete &=") {
auto hyperslab = HyperSlab();

// With detours, select everything, then reduce it to the first 2
// elements.
hyperslab |= RegularHyperSlab({0, 0}, {n, m / 2});
hyperslab |= RegularHyperSlab({0, 0}, {n, m / 2});
hyperslab |= RegularHyperSlab({0, m / 2}, {n, m - m / 2});
hyperslab |= RegularHyperSlab({0, 0}, {n, m});
hyperslab &= RegularHyperSlab({0, 0}, {1, 2});
hyperslab.apply(space);

indices = {{0, 0}, {0, 1}};

auto selected = dset.select(hyperslab).read<std::vector<int>>();
check_selected(selected, indices, x);
}

SECTION("empty |=") {
auto hyperslab = HyperSlab(RegularHyperSlab({0, 0}, {n, m}));
hyperslab &= RegularHyperSlab({0, 0}, {1, 2});
hyperslab &= RegularHyperSlab({3, 0}, {1, 2});

hyperslab |= RegularHyperSlab({0, 0}, {n, m / 2});
hyperslab |= RegularHyperSlab({0, 0}, {n, m / 2});
hyperslab |= RegularHyperSlab({0, m / 2}, {n, m - m / 2});
hyperslab |= RegularHyperSlab({0, 0}, {n, m});

hyperslab.apply(space);

auto selected = dset.select(hyperslab).read<std::vector<int>>();
check_selected(selected, indices, x);
}

SECTION("|= empty") {
auto hyperslab = HyperSlab();

hyperslab |= RegularHyperSlab({0, 0}, {1, 2});
hyperslab |= RegularHyperSlab({0, 0}, {1, 2});
hyperslab |= RegularHyperSlab({0, 0}, {0, 0});

hyperslab.apply(space);

indices = {{0, 0}, {0, 1}};

auto selected = dset.select(hyperslab).read<std::vector<int>>();
check_selected(selected, indices, x);
}
}

0 comments on commit a6e17bd

Please sign in to comment.