Skip to content

Commit

Permalink
Remove Status from class Subarray
Browse files Browse the repository at this point in the history 
This PR removes `Status` from all member functions of `class Subarray`. It also removes all occurrence of `Status` from `subarray/subarray.cc` except for the ones necessary to use `parallel_for`. There are a handful of changes that tagged along, although these were minimized to alleviate review. These other changes include (1) adding blocks to `if` statements without them, (2) removal of some inconsequential `const` declarations, (3) removal of some dead code.

In almost all cases, follow-on changes were not pursued. One exception is `Query::non_overlapping_ranges`, which is proxy for a subarray function; this PR changes its signature. Another was in the test helper `subarray_equiv`, which requires more rewriting that the rest of the call sites.
  • Loading branch information
@eric-hughes-tiledb
eric-hughes-tiledb committed Mar 28, 2024
1 parent 2e25ad2 commit 64b98a7
Show file tree
Hide file tree
Showing 24 changed files with 627 additions and 846 deletions.
4 changes: 2 additions & 2 deletions test/src/unit-CellSlabIter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ TEST_CASE_METHOD(
SubarrayRanges<uint64_t> ranges = {{5, 15, 3, 5, 11, 14}};
Layout subarray_layout = Layout::ROW_MAJOR;
create_subarray(array_->array_, ranges, subarray_layout, &subarray);
CHECK(subarray.compute_tile_coords<uint64_t>().ok());
CHECK_NOTHROW(subarray.compute_tile_coords<uint64_t>());

uint64_t tile_coords_0[] = {0};
uint64_t tile_coords_1[] = {1};
Expand Down Expand Up @@ -434,7 +434,7 @@ TEST_CASE_METHOD(
{1, 2, 5, 8},
};
create_subarray(array_->array_, ranges, subarray_layout, &subarray);
CHECK(subarray.compute_tile_coords<uint64_t>().ok());
CHECK_NOTHROW(subarray.compute_tile_coords<uint64_t>());

check_iter<uint64_t>(subarray, c_cell_slabs);

Expand Down
3 changes: 2 additions & 1 deletion test/src/unit-DenseTiler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ void DenseTilerFx::add_ranges(
uint64_t range_size,
tiledb::sm::Subarray* subarray) {
for (size_t i = 0; i < ranges.size(); ++i)
CHECK(subarray->add_range((uint32_t)i, Range(ranges[i], range_size)).ok());
CHECK_NOTHROW(
subarray->add_range((uint32_t)i, Range(ranges[i], range_size)));
}

void DenseTilerFx::open_array(
Expand Down
16 changes: 8 additions & 8 deletions test/src/unit-ReadCellSlabIter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ TEST_CASE_METHOD(
SubarrayRanges<uint64_t> ranges = {{5, 15}};
Layout subarray_layout = Layout::ROW_MAJOR;
create_subarray(array_->array_, ranges, subarray_layout, &subarray);
CHECK(subarray.compute_tile_coords<uint64_t>().ok());
CHECK_NOTHROW(subarray.compute_tile_coords<uint64_t>());

// Create result space tiles
std::vector<uint64_t> slice = {1, 100};
Expand Down Expand Up @@ -321,7 +321,7 @@ TEST_CASE_METHOD(
SubarrayRanges<uint64_t> ranges = {{5, 15}};
Layout subarray_layout = Layout::ROW_MAJOR;
create_subarray(array_->array_, ranges, subarray_layout, &subarray);
CHECK(subarray.compute_tile_coords<uint64_t>().ok());
CHECK_NOTHROW(subarray.compute_tile_coords<uint64_t>());

// Create result space tiles
std::vector<uint64_t> slice = {20, 30};
Expand Down Expand Up @@ -395,7 +395,7 @@ TEST_CASE_METHOD(
SubarrayRanges<uint64_t> ranges = {{5, 15, 3, 5, 11, 14}};
Layout subarray_layout = Layout::ROW_MAJOR;
create_subarray(array_->array_, ranges, subarray_layout, &subarray);
CHECK(subarray.compute_tile_coords<uint64_t>().ok());
CHECK_NOTHROW(subarray.compute_tile_coords<uint64_t>());

// Create result space tiles
std::vector<uint64_t> slice_1 = {5, 12};
Expand Down Expand Up @@ -480,7 +480,7 @@ TEST_CASE_METHOD(
SubarrayRanges<uint64_t> ranges = {{3, 15, 18, 20}};
Layout subarray_layout = Layout::ROW_MAJOR;
create_subarray(array_->array_, ranges, subarray_layout, &subarray);
CHECK(subarray.compute_tile_coords<uint64_t>().ok());
CHECK_NOTHROW(subarray.compute_tile_coords<uint64_t>());

// Create result space tiles
std::vector<uint64_t> slice = {3, 12};
Expand Down Expand Up @@ -704,7 +704,7 @@ TEST_CASE_METHOD(
Subarray subarray;
SubarrayRanges<uint64_t> ranges = {{2, 3}, {2, 6}};
create_subarray(array_->array_, ranges, subarray_layout, &subarray);
CHECK(subarray.compute_tile_coords<uint64_t>().ok());
CHECK_NOTHROW(subarray.compute_tile_coords<uint64_t>());

// Create result space tiles
std::vector<uint64_t> slice = {1, 6, 1, 6};
Expand Down Expand Up @@ -890,7 +890,7 @@ TEST_CASE_METHOD(
Subarray subarray;
SubarrayRanges<uint64_t> ranges = {{2, 3}, {2, 6}};
create_subarray(array_->array_, ranges, subarray_layout, &subarray);
CHECK(subarray.compute_tile_coords<uint64_t>().ok());
CHECK_NOTHROW(subarray.compute_tile_coords<uint64_t>());

// Create result space tiles
std::vector<uint64_t> slice = {6, 6, 6, 6};
Expand Down Expand Up @@ -1089,7 +1089,7 @@ TEST_CASE_METHOD(
Subarray subarray;
SubarrayRanges<uint64_t> ranges = {{2, 3}, {2, 6}};
create_subarray(array_->array_, ranges, subarray_layout, &subarray);
CHECK(subarray.compute_tile_coords<uint64_t>().ok());
CHECK_NOTHROW(subarray.compute_tile_coords<uint64_t>());

// Create result space tiles
std::vector<uint64_t> slice = {3, 6, 5, 6};
Expand Down Expand Up @@ -1332,7 +1332,7 @@ TEST_CASE_METHOD(
Subarray subarray;
SubarrayRanges<uint64_t> ranges = {{3, 5}, {2, 4, 5, 6}};
create_subarray(array_->array_, ranges, subarray_layout, &subarray);
CHECK(subarray.compute_tile_coords<uint64_t>().ok());
CHECK_NOTHROW(subarray.compute_tile_coords<uint64_t>());

// Create result space tiles
std::vector<uint64_t> slice_1 = {3, 5, 2, 4};
Expand Down
10 changes: 5 additions & 5 deletions test/src/unit-Subarray.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ TEST_CASE_METHOD(
SubarrayRanges<uint64_t> ranges = {{5, 7, 6, 15, 33, 43}};
Layout subarray_layout = Layout::ROW_MAJOR;
create_subarray(array_->array_, ranges, subarray_layout, &subarray);
CHECK(subarray.compute_tile_coords<uint64_t>().ok());
CHECK_NOTHROW(subarray.compute_tile_coords<uint64_t>());

// Prepare correct tile coordinates
std::vector<std::vector<uint8_t>> c_tile_coords;
Expand Down Expand Up @@ -248,7 +248,7 @@ TEST_CASE_METHOD(
SubarrayRanges<uint64_t> ranges = {{2, 2, 6, 10}, {2, 6, 5, 10}};
Layout subarray_layout = Layout::ROW_MAJOR;
create_subarray(array_->array_, ranges, subarray_layout, &subarray);
CHECK(subarray.compute_tile_coords<uint64_t>().ok());
CHECK_NOTHROW(subarray.compute_tile_coords<uint64_t>());

auto tile_coords = subarray.tile_coords();
CHECK(tile_coords == c_tile_coords);
Expand Down Expand Up @@ -310,11 +310,11 @@ TEST_CASE_METHOD(
subarray.crop_to_tile(&tile_coords[0], Layout::ROW_MAJOR);
const Range* range = nullptr;
CHECK(cropped_subarray.range_num() == 2);
CHECK(cropped_subarray.get_range(0, 0, &range).ok());
CHECK_NOTHROW(cropped_subarray.get_range(0, 0, &range));
CHECK(!memcmp(range->data(), &c_range_0_0[0], 2 * sizeof(uint64_t)));
CHECK(cropped_subarray.get_range(1, 0, &range).ok());
CHECK_NOTHROW(cropped_subarray.get_range(1, 0, &range));
CHECK(!memcmp(range->data(), &c_range_1_0[0], 2 * sizeof(uint64_t)));
CHECK(cropped_subarray.get_range(1, 1, &range).ok());
CHECK_NOTHROW(cropped_subarray.get_range(1, 1, &range));
CHECK(!memcmp(range->data(), &c_range_1_1[0], 2 * sizeof(uint64_t)));

close_array(ctx_, array_);
Expand Down
26 changes: 13 additions & 13 deletions test/src/unit-SubarrayPartitioner-sparse.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2287,7 +2287,7 @@ TEST_CASE_METHOD(
// Check unsplittable
tiledb::sm::Subarray subarray(
array->array_.get(), layout, &g_helper_stats, g_helper_logger());
CHECK(subarray.add_range(0, Range("bb", "bb"), true).ok());
CHECK_NOTHROW(subarray.add_range(0, Range("bb", "bb"), true));
ThreadPool tp(4);
Config config;
SubarrayPartitioner partitioner(
Expand Down Expand Up @@ -2315,15 +2315,15 @@ TEST_CASE_METHOD(
auto partition = partitioner.current();
CHECK(partition.range_num() == 1);
const Range* range = nullptr;
CHECK(partition.get_range(0, 0, &range).ok());
CHECK_NOTHROW(partition.get_range(0, 0, &range));
CHECK(range != nullptr);
CHECK(range->start_str() == std::string("bb", 2));
CHECK(range->end_str() == std::string("bb", 2));

// Check full
tiledb::sm::Subarray subarray_full(
array->array_.get(), layout, &g_helper_stats, g_helper_logger());
CHECK(subarray_full.add_range(0, Range("a", "bb"), true).ok());
CHECK_NOTHROW(subarray_full.add_range(0, Range("a", "bb"), true));
SubarrayPartitioner partitioner_full(
&config,
subarray_full,
Expand All @@ -2342,15 +2342,15 @@ TEST_CASE_METHOD(
CHECK(!unsplittable);
partition = partitioner_full.current();
CHECK(partition.range_num() == 1);
CHECK(partition.get_range(0, 0, &range).ok());
CHECK_NOTHROW(partition.get_range(0, 0, &range));
CHECK(range != nullptr);
CHECK(range->start_str() == std::string("a", 1));
CHECK(range->end_str() == std::string("bb", 2));

// Check split
tiledb::sm::Subarray subarray_split(
array->array_.get(), layout, &g_helper_stats, g_helper_logger());
CHECK(subarray_split.add_range(0, Range("a", "bb"), true).ok());
CHECK_NOTHROW(subarray_split.add_range(0, Range("a", "bb"), true));
SubarrayPartitioner partitioner_split(
&config,
subarray_split,
Expand All @@ -2370,15 +2370,15 @@ TEST_CASE_METHOD(
CHECK(!unsplittable);
partition = partitioner_split.current();
CHECK(partition.range_num() == 1);
CHECK(partition.get_range(0, 0, &range).ok());
CHECK_NOTHROW(partition.get_range(0, 0, &range));
CHECK(range != nullptr);
CHECK(range->start_str() == std::string("a", 1));
CHECK(range->end_str() == std::string("a\x7F", 2));
CHECK(partitioner_split.next(&unsplittable).ok());
CHECK(!unsplittable);
partition = partitioner_split.current();
CHECK(partition.range_num() == 1);
CHECK(partition.get_range(0, 0, &range).ok());
CHECK_NOTHROW(partition.get_range(0, 0, &range));
CHECK(range != nullptr);
CHECK(range->start_str() == std::string("b", 1));
CHECK(range->end_str() == std::string("bb", 2));
Expand All @@ -2387,7 +2387,7 @@ TEST_CASE_METHOD(
// Check no split 2 MBRs
tiledb::sm::Subarray subarray_no_split(
array->array_.get(), layout, &g_helper_stats, g_helper_logger());
CHECK(subarray_no_split.add_range(0, Range("bb", "cc"), true).ok());
CHECK_NOTHROW(subarray_no_split.add_range(0, Range("bb", "cc"), true));
SubarrayPartitioner partitioner_no_split(
&config,
subarray_no_split,
Expand All @@ -2408,15 +2408,15 @@ TEST_CASE_METHOD(
CHECK(!unsplittable);
partition = partitioner_no_split.current();
CHECK(partition.range_num() == 1);
CHECK(partition.get_range(0, 0, &range).ok());
CHECK_NOTHROW(partition.get_range(0, 0, &range));
CHECK(range != nullptr);
CHECK(range->start_str() == std::string("bb", 2));
CHECK(range->end_str() == std::string("cc", 2));

// Check split 2 MBRs
tiledb::sm::Subarray subarray_split_2(
array->array_.get(), layout, &g_helper_stats, g_helper_logger());
CHECK(subarray_split_2.add_range(0, Range("bb", "cc"), true).ok());
CHECK_NOTHROW(subarray_split_2.add_range(0, Range("bb", "cc"), true));
SubarrayPartitioner partitioner_split_2(
&config,
subarray_split_2,
Expand All @@ -2437,15 +2437,15 @@ TEST_CASE_METHOD(
CHECK(!unsplittable);
partition = partitioner_split_2.current();
CHECK(partition.range_num() == 1);
CHECK(partition.get_range(0, 0, &range).ok());
CHECK_NOTHROW(partition.get_range(0, 0, &range));
CHECK(range != nullptr);
CHECK(range->start_str() == std::string("bb", 2));
CHECK(range->end_str() == std::string("b\x7F", 2));
CHECK(partitioner_split_2.next(&unsplittable).ok());
CHECK(!unsplittable);
partition = partitioner_split_2.current();
CHECK(partition.range_num() == 1);
CHECK(partition.get_range(0, 0, &range).ok());
CHECK_NOTHROW(partition.get_range(0, 0, &range));
CHECK(range != nullptr);
CHECK(range->start_str() == std::string("c", 1));
CHECK(range->end_str() == std::string("cc", 2));
Expand Down Expand Up @@ -2554,7 +2554,7 @@ TEST_CASE_METHOD(

tiledb::sm::Subarray subarray(
array->array_.get(), layout, &g_helper_stats, g_helper_logger());
CHECK(subarray.add_range(0, Range("cc", "ccd"), true).ok());
CHECK_NOTHROW(subarray.add_range(0, Range("cc", "ccd"), true));
ThreadPool tp(4);
Config config;
SubarrayPartitioner partitioner(
Expand Down
12 changes: 6 additions & 6 deletions test/src/unit-cppapi-subarray.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,29 +467,29 @@ TEST_CASE(
SECTION("- Upper bound OOB") {
int range[] = {0, 100};
auto r = Range(&range[0], &range[1], sizeof(int));
CHECK(subarray.ptr().get()->subarray_->add_range_unsafe(0, r).ok());
CHECK_NOTHROW(subarray.ptr().get()->subarray_->add_range_unsafe(0, r));
}

SECTION("- Lower bound OOB") {
int range[] = {-1, 2};
auto r = Range(&range[0], &range[1], sizeof(int));
CHECK(subarray.ptr().get()->subarray_->add_range_unsafe(0, r).ok());
CHECK_NOTHROW(subarray.ptr().get()->subarray_->add_range_unsafe(0, r));
}

SECTION("- Second range OOB") {
int range[] = {1, 4};
auto r = Range(&range[0], &range[1], sizeof(int));
CHECK(subarray.ptr().get()->subarray_->add_range_unsafe(0, r).ok());
CHECK_NOTHROW(subarray.ptr().get()->subarray_->add_range_unsafe(0, r));
int range2[] = {10, 20};
auto r2 = Range(&range2[0], &range2[1], sizeof(int));
CHECK(subarray.ptr().get()->subarray_->add_range_unsafe(1, r2).ok());
CHECK_NOTHROW(subarray.ptr().get()->subarray_->add_range_unsafe(1, r2));
}

SECTION("- Valid ranges") {
int range[] = {0, 1};
auto r = Range(&range[0], &range[1], sizeof(int));
CHECK(subarray.ptr().get()->subarray_->add_range_unsafe(0, r).ok());
CHECK(subarray.ptr().get()->subarray_->add_range_unsafe(1, r).ok());
CHECK_NOTHROW(subarray.ptr().get()->subarray_->add_range_unsafe(0, r));
CHECK_NOTHROW(subarray.ptr().get()->subarray_->add_range_unsafe(1, r));
expected = TILEDB_OK;
}

Expand Down
Loading

0 comments on commit 64b98a7

Please sign in to comment.