From c85e738a77daa4e859a4691ee62e1fc5a5a42044 Mon Sep 17 00:00:00 2001 From: Tobias Fuchs Date: Fri, 27 Feb 2015 16:36:26 +0100 Subject: [PATCH] algorithms_cpp: unified behavior on empty input, added unit tests on empty and negative input ranges --- algorithms_cpp/include/embb/algorithms/internal/for_each-inl.h | 2 ++ algorithms_cpp/include/embb/algorithms/internal/merge_sort-inl.h | 6 +++--- algorithms_cpp/include/embb/algorithms/internal/quick_sort-inl.h | 4 +++- algorithms_cpp/include/embb/algorithms/internal/reduce-inl.h | 4 +++- algorithms_cpp/include/embb/algorithms/internal/scan-inl.h | 4 +++- algorithms_cpp/include/embb/algorithms/merge_sort.h | 5 +++++ algorithms_cpp/test/for_each_test.cc | 25 +++++++++++++++++++++++++ algorithms_cpp/test/merge_sort_test.cc | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++------------------------ algorithms_cpp/test/merge_sort_test.h | 2 +- algorithms_cpp/test/quick_sort_test.cc | 26 ++++++++++++++++++++++++++ algorithms_cpp/test/reduce_test.cc | 25 +++++++++++++++++++++++++ algorithms_cpp/test/scan_test.cc | 29 +++++++++++++++++++++++++++++ 12 files changed, 175 insertions(+), 31 deletions(-) diff --git a/algorithms_cpp/include/embb/algorithms/internal/for_each-inl.h b/algorithms_cpp/include/embb/algorithms/internal/for_each-inl.h index a6cd15f..8d9e034 100644 --- a/algorithms_cpp/include/embb/algorithms/internal/for_each-inl.h +++ b/algorithms_cpp/include/embb/algorithms/internal/for_each-inl.h @@ -107,6 +107,8 @@ void ForEachRecursive(RAI first, RAI last, Function unary, difference_type distance = std::distance(first, last); if (distance == 0) { return; + } else if (distance < 0) { + EMBB_THROW(embb::base::ErrorException, "Negative range for ForEach"); } unsigned int num_cores = policy.GetCoreCount(); if (num_cores == 0) { diff --git a/algorithms_cpp/include/embb/algorithms/internal/merge_sort-inl.h b/algorithms_cpp/include/embb/algorithms/internal/merge_sort-inl.h index 7e7b0f7..163f4ca 100644 --- a/algorithms_cpp/include/embb/algorithms/internal/merge_sort-inl.h +++ b/algorithms_cpp/include/embb/algorithms/internal/merge_sort-inl.h @@ -229,7 +229,9 @@ void MergeSort( functor_t; difference_type distance = std::distance(first, last); if (distance == 0) { - EMBB_THROW(embb::base::ErrorException, "Distance for ForEach is 0"); + return; + } else if (distance < 0) { + EMBB_THROW(embb::base::ErrorException, "Negative range for MergeSort"); } unsigned int num_cores = policy.GetCoreCount(); if (num_cores == 0) { @@ -264,8 +266,6 @@ void MergeSort( task.Wait(MTAPI_INFINITE); } -// @NOTE: Why is there no type guard for RAI? - } // namespace algorithms } // namespace embb diff --git a/algorithms_cpp/include/embb/algorithms/internal/quick_sort-inl.h b/algorithms_cpp/include/embb/algorithms/internal/quick_sort-inl.h index 4ce296f..6df4caa 100644 --- a/algorithms_cpp/include/embb/algorithms/internal/quick_sort-inl.h +++ b/algorithms_cpp/include/embb/algorithms/internal/quick_sort-inl.h @@ -194,8 +194,10 @@ void QuickSort(RAI first, RAI last, ComparisonFunction comparison, embb::mtapi::Node& node = embb::mtapi::Node::GetInstance(); typedef typename std::iterator_traits::difference_type difference_type; difference_type distance = std::distance(first, last); - if (distance <= 0) { + if (distance == 0) { return; + } else if (distance < 0) { + EMBB_THROW(embb::base::ErrorException, "Negative range for QuickSort"); } unsigned int num_cores = policy.GetCoreCount(); if (num_cores == 0) { diff --git a/algorithms_cpp/include/embb/algorithms/internal/reduce-inl.h b/algorithms_cpp/include/embb/algorithms/internal/reduce-inl.h index e8fb907..86892a2 100644 --- a/algorithms_cpp/include/embb/algorithms/internal/reduce-inl.h +++ b/algorithms_cpp/include/embb/algorithms/internal/reduce-inl.h @@ -129,7 +129,9 @@ ReturnType ReduceRecursive(RAI first, RAI last, ReturnType neutral, typedef typename std::iterator_traits::difference_type difference_type; difference_type distance = std::distance(first, last); if (distance == 0) { - EMBB_THROW(embb::base::ErrorException, "Distance for Reduce is 0"); + return neutral; + } else if (distance < 0) { + EMBB_THROW(embb::base::ErrorException, "Negative range for Reduce"); } unsigned int num_cores = policy.GetCoreCount(); if (num_cores == 0) { diff --git a/algorithms_cpp/include/embb/algorithms/internal/scan-inl.h b/algorithms_cpp/include/embb/algorithms/internal/scan-inl.h index ab2a21b..872cfd9 100644 --- a/algorithms_cpp/include/embb/algorithms/internal/scan-inl.h +++ b/algorithms_cpp/include/embb/algorithms/internal/scan-inl.h @@ -173,8 +173,10 @@ void ScanIteratorCheck(RAIIn first, RAIIn last, RAIOut output_iterator, std::random_access_iterator_tag) { typedef typename std::iterator_traits::difference_type difference_type; difference_type distance = std::distance(first, last); - if (distance <= 0) { + if (distance == 0) { return; + } else if (distance < 0) { + EMBB_THROW(embb::base::ErrorException, "Negative range for Scan"); } unsigned int num_cores = policy.GetCoreCount(); if (num_cores == 0) { diff --git a/algorithms_cpp/include/embb/algorithms/merge_sort.h b/algorithms_cpp/include/embb/algorithms/merge_sort.h index d3de332..432f767 100644 --- a/algorithms_cpp/include/embb/algorithms/merge_sort.h +++ b/algorithms_cpp/include/embb/algorithms/merge_sort.h @@ -167,6 +167,11 @@ void MergeSortAllocate( typedef base::Allocation Alloc; typename std::iterator_traits::difference_type distance = last - first; typedef typename std::iterator_traits::value_type value_type; + if (distance == 0) { + return; + } else if (distance < 0) { + EMBB_THROW(embb::base::ErrorException, "Negative range for MergeSort"); + } value_type* temporary = static_cast( Alloc::Allocate(distance * sizeof(value_type))); EMBB_TRY { diff --git a/algorithms_cpp/test/for_each_test.cc b/algorithms_cpp/test/for_each_test.cc index 3a0e303..3ca3081 100644 --- a/algorithms_cpp/test/for_each_test.cc +++ b/algorithms_cpp/test/for_each_test.cc @@ -211,6 +211,31 @@ void ForEachTest::TestPolicy() { for (size_t i = 0; i < count; i++) { PT_EXPECT_EQ(vector[i], init[i]*init[i]); } + + // ForEach on empty list should not throw: + ForEach(vector.begin(), vector.begin(), Square()); + +#ifdef EMBB_USE_EXCEPTIONS + bool empty_core_set_thrown = false; + try { + ForEach(vector.begin(), vector.end(), Square(), ExecutionPolicy(false)); + } + catch (embb::base::ErrorException &) { + empty_core_set_thrown = true; + } + PT_EXPECT_MSG(empty_core_set_thrown, + "Empty core set should throw ErrorException"); + bool negative_range_thrown = false; + try { + std::vector::iterator second = vector.begin() + 1; + ForEach(second, vector.begin(), Square()); + } + catch (embb::base::ErrorException &) { + negative_range_thrown = true; + } + PT_EXPECT_MSG(negative_range_thrown, + "Negative range should throw ErrorException"); +#endif } void ForEachTest::StressTest() { diff --git a/algorithms_cpp/test/merge_sort_test.cc b/algorithms_cpp/test/merge_sort_test.cc index cbcbf0b..d7b1170 100644 --- a/algorithms_cpp/test/merge_sort_test.cc +++ b/algorithms_cpp/test/merge_sort_test.cc @@ -156,28 +156,28 @@ void MergeSortTest::TestRanges() { } } -//void MergeSortTest::TestBlockSizes() { -// using embb::algorithms::MergeSortAllocate; -// using embb::algorithms::ExecutionPolicy; -// size_t count = 4; -// std::vector init(count); -// std::vector vector(count); -// std::vector vector_copy(count); -// for (size_t i = count - 1; i > 0; i--) { -// init[i] = static_cast(i+2); -// } -// vector_copy = init; -// std::sort(vector_copy.begin(), vector_copy.end()); -// -// for (size_t block_size = 1; block_size < count + 2; block_size++) { -// vector = init; -// MergeSortAllocate(vector.begin(), vector.end(), std::less(), -// ExecutionPolicy(), block_size); -// for (size_t i = 0; i < count; i++) { -// PT_EXPECT_EQ(vector[i], vector_copy[i]); -// } -// } -//} +void MergeSortTest::TestBlockSizes() { + using embb::algorithms::MergeSortAllocate; + using embb::mtapi::ExecutionPolicy; + size_t count = 4; + std::vector init(count); + std::vector vector(count); + std::vector vector_copy(count); + for (size_t i = count - 1; i > 0; i--) { + init[i] = static_cast(i+2); + } + vector_copy = init; + std::sort(vector_copy.begin(), vector_copy.end()); + + for (size_t block_size = 1; block_size < count + 2; block_size++) { + vector = init; + MergeSortAllocate(vector.begin(), vector.end(), std::less(), + ExecutionPolicy(), block_size); + for (size_t i = 0; i < count; i++) { + PT_EXPECT_EQ(vector[i], vector_copy[i]); + } + } +} void MergeSortTest::TestPolicy() { using embb::algorithms::MergeSortAllocate; @@ -201,17 +201,43 @@ void MergeSortTest::TestPolicy() { vector = init; MergeSortAllocate(vector.begin(), vector.end(), std::less(), - ExecutionPolicy(true)); + ExecutionPolicy(true)); for (size_t i = 0; i < count; i++) { PT_EXPECT_EQ(vector_copy[i], vector[i]); } vector = init; MergeSortAllocate(vector.begin(), vector.end(), std::less(), - ExecutionPolicy(true, 1)); + ExecutionPolicy(true, 1)); for (size_t i = 0; i < count; i++) { PT_EXPECT_EQ(vector_copy[i], vector[i]); } + + // MergeSort on empty list should not throw: + MergeSortAllocate(vector.begin(), vector.begin(), std::less()); + +#ifdef EMBB_USE_EXCEPTIONS + bool empty_core_set_thrown = false; + try { + MergeSortAllocate(vector.begin(), vector.end(), std::less(), + ExecutionPolicy(false)); + } + catch (embb::base::ErrorException &) { + empty_core_set_thrown = true; + } + PT_EXPECT_MSG(empty_core_set_thrown, + "Empty core set should throw ErrorException"); + bool negative_range_thrown = false; + try { + std::vector::iterator second = vector.begin() + 1; + MergeSortAllocate(second, vector.begin(), std::less()); + } + catch (embb::base::ErrorException &) { + negative_range_thrown = true; + } + PT_EXPECT_MSG(negative_range_thrown, + "Negative range should throw ErrorException"); +#endif } void MergeSortTest::StressTest() { diff --git a/algorithms_cpp/test/merge_sort_test.h b/algorithms_cpp/test/merge_sort_test.h index 10545fb..7dbc8b0 100644 --- a/algorithms_cpp/test/merge_sort_test.h +++ b/algorithms_cpp/test/merge_sort_test.h @@ -58,7 +58,7 @@ class MergeSortTest : public partest::TestCase { /** * Tests various block sizes for the workers. */ - //void TestBlockSizes(); + void TestBlockSizes(); /** * Tests setting policies (without checking their actual execution). diff --git a/algorithms_cpp/test/quick_sort_test.cc b/algorithms_cpp/test/quick_sort_test.cc index 8f724ca..863b278 100644 --- a/algorithms_cpp/test/quick_sort_test.cc +++ b/algorithms_cpp/test/quick_sort_test.cc @@ -218,6 +218,32 @@ void QuickSortTest::TestPolicy() { for (size_t i = 0; i < count; i++) { PT_EXPECT_EQ(vector_copy[i], vector[i]); } + + // MergeSort on empty list should not throw: + QuickSort(vector.begin(), vector.begin(), std::less()); + +#ifdef EMBB_USE_EXCEPTIONS + bool empty_core_set_thrown = false; + try { + QuickSort(vector.begin(), vector.end(), std::less(), + ExecutionPolicy(false)); + } + catch (embb::base::ErrorException &) { + empty_core_set_thrown = true; + } + PT_EXPECT_MSG(empty_core_set_thrown, + "Empty core set should throw ErrorException"); + bool negative_range_thrown = false; + try { + std::vector::iterator second = vector.begin() + 1; + QuickSort(second, vector.begin(), std::less()); + } + catch (embb::base::ErrorException &) { + negative_range_thrown = true; + } + PT_EXPECT_MSG(negative_range_thrown, + "Negative range should throw ErrorException"); +#endif } void QuickSortTest::StressTest() { diff --git a/algorithms_cpp/test/reduce_test.cc b/algorithms_cpp/test/reduce_test.cc index 2ebebe5..c8ab5ae 100644 --- a/algorithms_cpp/test/reduce_test.cc +++ b/algorithms_cpp/test/reduce_test.cc @@ -181,6 +181,31 @@ void ReduceTest::TestPolicy() { Identity(), ExecutionPolicy(true)), sum); PT_EXPECT_EQ(Reduce(vector.begin(), vector.end(), 0, std::plus(), Identity(), ExecutionPolicy(true, 1)), sum); + // Empty list should return neutral element: + PT_EXPECT_EQ(Reduce(vector.begin(), vector.begin(), 41, std::plus(), + Identity(), ExecutionPolicy(true, 1)), 41); +#ifdef EMBB_USE_EXCEPTIONS + bool empty_core_set_thrown = false; + try { + Reduce(vector.begin(), vector.end(), 0, + std::plus(), Identity(), + ExecutionPolicy(false)); + } catch (embb::base::ErrorException &) { + empty_core_set_thrown = true; + } + PT_EXPECT_MSG(empty_core_set_thrown, + "Empty core set should throw ErrorException"); + bool negative_range_thrown = false; + try { + std::vector::iterator second = vector.begin() + 1; + Reduce(second, vector.begin(), 0, std::plus()); + } + catch (embb::base::ErrorException &) { + negative_range_thrown = true; + } + PT_EXPECT_MSG(negative_range_thrown, + "Negative range should throw ErrorException"); +#endif } void ReduceTest::StressTest() { diff --git a/algorithms_cpp/test/scan_test.cc b/algorithms_cpp/test/scan_test.cc index e88effd..e902a81 100644 --- a/algorithms_cpp/test/scan_test.cc +++ b/algorithms_cpp/test/scan_test.cc @@ -290,6 +290,35 @@ void ScanTest::TestPolicy() { expected += vector[i]; PT_EXPECT_EQ(expected, outputVector[i]); } + // Empty list should not throw and not change output: + outputVector = init; + std::vector::iterator out_it = outputVector.begin(); + Scan(vector.begin(), vector.begin(), out_it, 0, std::plus()); + PT_EXPECT(out_it == outputVector.begin()); + +#ifdef EMBB_USE_EXCEPTIONS + bool empty_core_set_thrown = false; + try { + Scan(vector.begin(), vector.end(), outputVector.begin(), + 0, std::plus(), Identity(), + ExecutionPolicy(false)); + } + catch (embb::base::ErrorException &) { + empty_core_set_thrown = true; + } + PT_EXPECT_MSG(empty_core_set_thrown, + "Empty core set should throw ErrorException"); + bool negative_range_thrown = false; + try { + std::vector::iterator second = vector.begin() + 1; + Scan(second, vector.begin(), outputVector.begin(), 0, std::plus()); + } + catch (embb::base::ErrorException &) { + negative_range_thrown = true; + } + PT_EXPECT_MSG(negative_range_thrown, + "Negative range should throw ErrorException"); +#endif } void ScanTest::StressTest() { -- libgit2 0.26.0