From 108a19e8d0f31a11f05a1a6bd91fbe176e7d5cbc Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Thu, 3 Jul 2025 12:45:39 -0700 Subject: [PATCH 01/16] limit the maximum number of iterations for binary search --- cxx/isce3/core/Utilities.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cxx/isce3/core/Utilities.h b/cxx/isce3/core/Utilities.h index 5c29cb221..526266c28 100644 --- a/cxx/isce3/core/Utilities.h +++ b/cxx/isce3/core/Utilities.h @@ -339,7 +339,10 @@ namespace isce3 { namespace core { int left = 0; int right = array.size() - 1; int index; - while (left <= right) { + + long long counter = 0; + long long MAX_NUM_ITERATIONS = 1e11; + while (left <= right and counter < MAX_NUM_ITERATIONS) { const int middle = static_cast(std::round(0.5 * (left + right))); if (left == (right - 1)) { index = left; From d3965623b42b53ad0dea3bab5249d86fe96df9f4 Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Thu, 3 Jul 2025 12:53:10 -0700 Subject: [PATCH 02/16] limit the maximum number of iterations for binary search --- cxx/isce3/core/Utilities.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cxx/isce3/core/Utilities.h b/cxx/isce3/core/Utilities.h index 526266c28..ce0265d89 100644 --- a/cxx/isce3/core/Utilities.h +++ b/cxx/isce3/core/Utilities.h @@ -340,9 +340,9 @@ namespace isce3 { namespace core { int right = array.size() - 1; int index; - long long counter = 0; + long long count = 0; long long MAX_NUM_ITERATIONS = 1e11; - while (left <= right and counter < MAX_NUM_ITERATIONS) { + while (left <= right and count < MAX_NUM_ITERATIONS) { const int middle = static_cast(std::round(0.5 * (left + right))); if (left == (right - 1)) { index = left; @@ -353,6 +353,7 @@ namespace isce3 { namespace core { } else if (array[middle] > value) { right = middle; } + count++; } index = left; return index; From e19052c137568469e15ee9cbaa01ac9fd96e371b Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Thu, 3 Jul 2025 15:35:19 -0700 Subject: [PATCH 03/16] raise an error if array contains NaN --- cxx/isce3/core/Utilities.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cxx/isce3/core/Utilities.h b/cxx/isce3/core/Utilities.h index ce0265d89..e5370c09e 100644 --- a/cxx/isce3/core/Utilities.h +++ b/cxx/isce3/core/Utilities.h @@ -344,13 +344,17 @@ namespace isce3 { namespace core { long long MAX_NUM_ITERATIONS = 1e11; while (left <= right and count < MAX_NUM_ITERATIONS) { const int middle = static_cast(std::round(0.5 * (left + right))); + const auto middle_value = array[middle]; + if (std::isnan(middle_value)) { + throw std::invalid_argument("input array may not contain NaN values"); + } if (left == (right - 1)) { index = left; return index; } - if (array[middle] <= value) { + if (middle_value <= value) { left = middle; - } else if (array[middle] > value) { + } else if (middle_value > value) { right = middle; } count++; From 15d38d30b18a931f4268728887839b2790940af1 Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Thu, 3 Jul 2025 23:21:38 -0700 Subject: [PATCH 04/16] add imports --- cxx/isce3/core/Utilities.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cxx/isce3/core/Utilities.h b/cxx/isce3/core/Utilities.h index e5370c09e..274d84bd2 100644 --- a/cxx/isce3/core/Utilities.h +++ b/cxx/isce3/core/Utilities.h @@ -16,6 +16,8 @@ #include #include #include +#include +#include // Macro wrappers to check vector lengths // (adds calling function and variable name information to the exception) From feef376d97df4485b7a3bb9b3cb81b8b4497a4d4 Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Thu, 3 Jul 2025 23:22:29 -0700 Subject: [PATCH 05/16] simplify code --- cxx/isce3/core/Utilities.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cxx/isce3/core/Utilities.h b/cxx/isce3/core/Utilities.h index 274d84bd2..199900e99 100644 --- a/cxx/isce3/core/Utilities.h +++ b/cxx/isce3/core/Utilities.h @@ -356,7 +356,7 @@ namespace isce3 { namespace core { } if (middle_value <= value) { left = middle; - } else if (middle_value > value) { + } else { right = middle; } count++; From 1b2e7e9a4c7e43dc561e10870d2d2472dfc23c9d Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Mon, 7 Jul 2025 13:27:47 -0700 Subject: [PATCH 06/16] limit the number of iterations in the binary search --- cxx/isce3/core/Utilities.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cxx/isce3/core/Utilities.h b/cxx/isce3/core/Utilities.h index 199900e99..3e6a80b27 100644 --- a/cxx/isce3/core/Utilities.h +++ b/cxx/isce3/core/Utilities.h @@ -343,8 +343,7 @@ namespace isce3 { namespace core { int index; long long count = 0; - long long MAX_NUM_ITERATIONS = 1e11; - while (left <= right and count < MAX_NUM_ITERATIONS) { + while (left <= right) { const int middle = static_cast(std::round(0.5 * (left + right))); const auto middle_value = array[middle]; if (std::isnan(middle_value)) { @@ -360,6 +359,10 @@ namespace isce3 { namespace core { right = middle; } count++; + if (count >= array.size()) { + throw std::runtime_error( + "Binary search failed to converge within the allowed iterations."); + } } index = left; return index; From a3684dac71ecbd081fb7aa7c23a3e892934fb9e4 Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Tue, 8 Jul 2025 13:28:25 -0700 Subject: [PATCH 07/16] use size_t instead of long long --- cxx/isce3/core/Utilities.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cxx/isce3/core/Utilities.h b/cxx/isce3/core/Utilities.h index 3e6a80b27..4d181cbc9 100644 --- a/cxx/isce3/core/Utilities.h +++ b/cxx/isce3/core/Utilities.h @@ -342,7 +342,7 @@ namespace isce3 { namespace core { int right = array.size() - 1; int index; - long long count = 0; + std::size_t count = 0; while (left <= right) { const int middle = static_cast(std::round(0.5 * (left + right))); const auto middle_value = array[middle]; @@ -359,7 +359,7 @@ namespace isce3 { namespace core { right = middle; } count++; - if (count >= array.size()) { + if (count > array.size()) { throw std::runtime_error( "Binary search failed to converge within the allowed iterations."); } From 96915ae696d3035b09de10c4eb802822fbc1ad3a Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Wed, 9 Jul 2025 11:58:25 -0700 Subject: [PATCH 08/16] update the binary search `binarySearch()` --- cxx/isce3/core/Utilities.h | 40 +++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/cxx/isce3/core/Utilities.h b/cxx/isce3/core/Utilities.h index 4d181cbc9..a4f499c1e 100644 --- a/cxx/isce3/core/Utilities.h +++ b/cxx/isce3/core/Utilities.h @@ -334,38 +334,52 @@ namespace isce3 { namespace core { } } - /** Searches array for index closest to provided value */ + /** Searches array for index closest to provided value + * + * Assumes the array is sorted in ascending order and contains no NaNs. + */ inline int binarySearch(const std::valarray & array, double value) { - + + if (array.size() == 0) { + throw std::invalid_argument("input array must contain at least 1 element"); + } + if (array.size() == 1) { + return 0; + } + // Do the binary search int left = 0; int right = array.size() - 1; - int index; std::size_t count = 0; - while (left <= right) { + + // `max_iter` provides a safeguard against unexpected behavior. + // The loop is guaranteed to converge with correct input, but we include + // this check as a safe measure for unexpected edge cases. + std::size_t max_iter = std::ceil(std::log2(array.size())) + 1; + while (left + 1 < right) { const int middle = static_cast(std::round(0.5 * (left + right))); const auto middle_value = array[middle]; if (std::isnan(middle_value)) { throw std::invalid_argument("input array may not contain NaN values"); } - if (left == (right - 1)) { - index = left; - return index; - } - if (middle_value <= value) { + if (middle_value < value) { left = middle; } else { right = middle; } - count++; - if (count > array.size()) { + if (++count > max_iter) { throw std::runtime_error( "Binary search failed to converge within the allowed iterations."); } } - index = left; - return index; + + // Return the closest of left and right + if (std::abs(array[left] - value) <= std::abs(array[right] - value)) { + return left; + } + + return right; } /** Clip a number between an upper and lower range (implements std::clamp for older GCC) */ From 8893ba8ba9088203e4db9086d04960c23a470599 Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Wed, 9 Jul 2025 13:51:30 -0700 Subject: [PATCH 09/16] update the binary search `binarySearch()` --- cxx/isce3/core/Utilities.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cxx/isce3/core/Utilities.h b/cxx/isce3/core/Utilities.h index a4f499c1e..c197d6c83 100644 --- a/cxx/isce3/core/Utilities.h +++ b/cxx/isce3/core/Utilities.h @@ -357,6 +357,7 @@ namespace isce3 { namespace core { // The loop is guaranteed to converge with correct input, but we include // this check as a safe measure for unexpected edge cases. std::size_t max_iter = std::ceil(std::log2(array.size())) + 1; + // std::cout << "binary search" << std::endl; while (left + 1 < right) { const int middle = static_cast(std::round(0.5 * (left + right))); const auto middle_value = array[middle]; @@ -372,6 +373,11 @@ namespace isce3 { namespace core { throw std::runtime_error( "Binary search failed to converge within the allowed iterations."); } + // std::cout << "count:" << count << std::endl; + } + + if (std::isnan(array[left]) || std::isnan(array[right])) { + throw std::invalid_argument("input array may not contain NaN values"); } // Return the closest of left and right From 0da209aced4de20fdb953956475b8ee37ed6ee02 Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Wed, 9 Jul 2025 13:51:45 -0700 Subject: [PATCH 10/16] add unit tests for `binarySearch()` --- tests/cxx/isce3/Sources.cmake | 1 + .../cxx/isce3/core/utilities/binarySearch.cpp | 50 +++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 tests/cxx/isce3/core/utilities/binarySearch.cpp diff --git a/tests/cxx/isce3/Sources.cmake b/tests/cxx/isce3/Sources.cmake index 80c5f37cb..aae2006be 100644 --- a/tests/cxx/isce3/Sources.cmake +++ b/tests/cxx/isce3/Sources.cmake @@ -23,6 +23,7 @@ core/projections/utm.cpp core/serialization/serializeAttitude.cpp core/serialization/serializeDoppler.cpp core/serialization/serializeOrbit.cpp +core/utilities/binarySearch.cpp fft/fft.cpp fft/fftplan.cpp fft/fftutil.cpp diff --git a/tests/cxx/isce3/core/utilities/binarySearch.cpp b/tests/cxx/isce3/core/utilities/binarySearch.cpp new file mode 100644 index 000000000..826963005 --- /dev/null +++ b/tests/cxx/isce3/core/utilities/binarySearch.cpp @@ -0,0 +1,50 @@ +#include + +#include + +using isce3::core::binarySearch; + +TEST(BinarySearchTest, SingleElementArray) { + std::valarray arr = {2.0}; + EXPECT_EQ(binarySearch(arr, 0.0), 0); + EXPECT_EQ(binarySearch(arr, 100.0), 0); +} + +TEST(BinarySearchTest, ClosestMatchExact) { + std::valarray arr = {1.0, 3.0, 5.0, 7.0}; + EXPECT_EQ(binarySearch(arr, 3.0), 1); + EXPECT_EQ(binarySearch(arr, 7.0), 3); +} + +TEST(BinarySearchTest, ClosestMatchInBetween) { + std::valarray arr = {1.0, 3.0, 5.0, 7.0}; + EXPECT_EQ(binarySearch(arr, 2.1), 1); // closer to `3` + EXPECT_EQ(binarySearch(arr, 4.1), 2); // closer to `5` + EXPECT_EQ(binarySearch(arr, 6.4), 3); // closer to `7` +} + +TEST(BinarySearchTest, ValueBeforeFirstElement) { + std::valarray arr = {1.0, 2.0, 3.0}; + EXPECT_EQ(binarySearch(arr, -1.0), 0); // `-1` closest to `1` +} + +TEST(BinarySearchTest, ValueAfterLastElement) { + std::valarray arr = {1.0, 2.0, 3.0}; + EXPECT_EQ(binarySearch(arr, 5.0), 2); // `5` closest to `3` +} + +TEST(BinarySearchTest, ThrowsOnEmptyArray) { + std::valarray arr; + EXPECT_THROW(binarySearch(arr, 0.0), std::invalid_argument); +} + +TEST(BinarySearchTest, ThrowsOnNaN) { + std::valarray arr = {1.0, 2.0, NAN, 4.0}; + EXPECT_THROW(binarySearch(arr, 3.0), std::invalid_argument); +} + +int main(int argc, char * argv[]) +{ + testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} From e8f5f7f1a5041afb7695c773a20dd6681d150da9 Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Wed, 9 Jul 2025 13:58:12 -0700 Subject: [PATCH 11/16] remove comments --- cxx/isce3/core/Utilities.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/cxx/isce3/core/Utilities.h b/cxx/isce3/core/Utilities.h index c197d6c83..796022279 100644 --- a/cxx/isce3/core/Utilities.h +++ b/cxx/isce3/core/Utilities.h @@ -357,7 +357,6 @@ namespace isce3 { namespace core { // The loop is guaranteed to converge with correct input, but we include // this check as a safe measure for unexpected edge cases. std::size_t max_iter = std::ceil(std::log2(array.size())) + 1; - // std::cout << "binary search" << std::endl; while (left + 1 < right) { const int middle = static_cast(std::round(0.5 * (left + right))); const auto middle_value = array[middle]; @@ -373,7 +372,6 @@ namespace isce3 { namespace core { throw std::runtime_error( "Binary search failed to converge within the allowed iterations."); } - // std::cout << "count:" << count << std::endl; } if (std::isnan(array[left]) || std::isnan(array[right])) { From 563fea9e31e65418f30e9c75e65aeb97d2b4b79b Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Wed, 9 Jul 2025 20:00:23 -0700 Subject: [PATCH 12/16] add argument `always_pick_left` to `binarySearch()` --- cxx/isce3/core/Utilities.h | 9 +++- cxx/isce3/geometry/Topo.cpp | 4 +- .../cxx/isce3/core/utilities/binarySearch.cpp | 50 +++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/cxx/isce3/core/Utilities.h b/cxx/isce3/core/Utilities.h index 796022279..0fca46e99 100644 --- a/cxx/isce3/core/Utilities.h +++ b/cxx/isce3/core/Utilities.h @@ -338,7 +338,8 @@ namespace isce3 { namespace core { * * Assumes the array is sorted in ascending order and contains no NaNs. */ - inline int binarySearch(const std::valarray & array, double value) { + inline int binarySearch(const std::valarray & array, double value, + const bool always_pick_left = false) { if (array.size() == 0) { throw std::invalid_argument("input array must contain at least 1 element"); @@ -358,7 +359,7 @@ namespace isce3 { namespace core { // this check as a safe measure for unexpected edge cases. std::size_t max_iter = std::ceil(std::log2(array.size())) + 1; while (left + 1 < right) { - const int middle = static_cast(std::round(0.5 * (left + right))); + const int middle = left + (right - left) / 2; const auto middle_value = array[middle]; if (std::isnan(middle_value)) { throw std::invalid_argument("input array may not contain NaN values"); @@ -374,6 +375,10 @@ namespace isce3 { namespace core { } } + if (always_pick_left) { + return left; + } + if (std::isnan(array[left]) || std::isnan(array[right])) { throw std::invalid_argument("input array may not contain NaN values"); } diff --git a/cxx/isce3/geometry/Topo.cpp b/cxx/isce3/geometry/Topo.cpp index 813425690..5bdbdfb4d 100644 --- a/cxx/isce3/geometry/Topo.cpp +++ b/cxx/isce3/geometry/Topo.cpp @@ -812,7 +812,9 @@ setLayoverShadow(TopoLayers& layers, DEMInterpolator& demInterp, // Compute nearest ctrack index for current ctrackGrid value const double crossTrack = ctrackGrid[i]; - int k = isce3::core::binarySearch(ctrack, crossTrack); + const bool always_pick_left = true; + int k = isce3::core::binarySearch(ctrack, crossTrack, always_pick_left); + // Adjust edges if necessary if (k == (width - 1)) { k = width - 2; diff --git a/tests/cxx/isce3/core/utilities/binarySearch.cpp b/tests/cxx/isce3/core/utilities/binarySearch.cpp index 826963005..90fbd9545 100644 --- a/tests/cxx/isce3/core/utilities/binarySearch.cpp +++ b/tests/cxx/isce3/core/utilities/binarySearch.cpp @@ -10,12 +10,27 @@ TEST(BinarySearchTest, SingleElementArray) { EXPECT_EQ(binarySearch(arr, 100.0), 0); } +TEST(BinarySearchTest, SingleElementArrayPickLeft) { + const bool always_pick_left = true; + std::valarray arr = {2.0}; + EXPECT_EQ(binarySearch(arr, 0.0), 0, always_pick_left); + EXPECT_EQ(binarySearch(arr, 100.0), 0, always_pick_left); +} + + TEST(BinarySearchTest, ClosestMatchExact) { std::valarray arr = {1.0, 3.0, 5.0, 7.0}; EXPECT_EQ(binarySearch(arr, 3.0), 1); EXPECT_EQ(binarySearch(arr, 7.0), 3); } +TEST(BinarySearchTest, ClosestMatchExactPickLeft) { + const bool always_pick_left = true; + std::valarray arr = {1.0, 3.0, 5.0, 7.0}; + EXPECT_EQ(binarySearch(arr, 3.0), 1, always_pick_left); + EXPECT_EQ(binarySearch(arr, 7.0), 3, always_pick_left); +} + TEST(BinarySearchTest, ClosestMatchInBetween) { std::valarray arr = {1.0, 3.0, 5.0, 7.0}; EXPECT_EQ(binarySearch(arr, 2.1), 1); // closer to `3` @@ -23,16 +38,51 @@ TEST(BinarySearchTest, ClosestMatchInBetween) { EXPECT_EQ(binarySearch(arr, 6.4), 3); // closer to `7` } +TEST(BinarySearchTest, ClosestMatchInBetweenPickLeft) { + const bool always_pick_left = true; + std::valarray arr = {1.0, 3.0, 5.0, 7.0}; + EXPECT_EQ(binarySearch(arr, 2.1), 0, always_pick_left); + EXPECT_EQ(binarySearch(arr, 4.1), 1, always_pick_left); + EXPECT_EQ(binarySearch(arr, 6.4), 2, always_pick_left); +} + TEST(BinarySearchTest, ValueBeforeFirstElement) { std::valarray arr = {1.0, 2.0, 3.0}; EXPECT_EQ(binarySearch(arr, -1.0), 0); // `-1` closest to `1` } +TEST(BinarySearchTest, ValueBeforeFirstElementPickLeft) { + const bool always_pick_left = true; + std::valarray arr = {1.0, 2.0, 3.0}; + EXPECT_EQ(binarySearch(arr, -1.0), 0, always_pick_left); +} + TEST(BinarySearchTest, ValueAfterLastElement) { std::valarray arr = {1.0, 2.0, 3.0}; EXPECT_EQ(binarySearch(arr, 5.0), 2); // `5` closest to `3` } +TEST(BinarySearchTest, ValueAfterLastElementPickLeft) { + const bool always_pick_left = true; + std::valarray arr = {1.0, 2.0, 3.0}; + EXPECT_EQ(binarySearch(arr, 5.0), 2, always_pick_left); +} + +TEST(BinarySearchTest, EqualDistanceChoosesLeft) { + std::valarray arr = {1.0, 2.0, 3.0, 4.0}; + + // `2` and `3` equally distant; picks left (`2`) + EXPECT_EQ(binarySearch(arr, 2.5), 1); +} + +TEST(BinarySearchTest, EqualDistanceChoosesLeftPickLeft) { + const bool always_pick_left = true; + std::valarray arr = {1.0, 2.0, 3.0, 4.0}; + + // `2` and `3` equally distant; picks left (`2`) + EXPECT_EQ(binarySearch(arr, 2.5), 1, always_pick_left); +} + TEST(BinarySearchTest, ThrowsOnEmptyArray) { std::valarray arr; EXPECT_THROW(binarySearch(arr, 0.0), std::invalid_argument); From 00a33f785c39887aef67d0a45d26365b4c4793e6 Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Wed, 9 Jul 2025 20:07:55 -0700 Subject: [PATCH 13/16] fix argument position --- .../cxx/isce3/core/utilities/binarySearch.cpp | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/cxx/isce3/core/utilities/binarySearch.cpp b/tests/cxx/isce3/core/utilities/binarySearch.cpp index 90fbd9545..ef067ad2d 100644 --- a/tests/cxx/isce3/core/utilities/binarySearch.cpp +++ b/tests/cxx/isce3/core/utilities/binarySearch.cpp @@ -13,8 +13,8 @@ TEST(BinarySearchTest, SingleElementArray) { TEST(BinarySearchTest, SingleElementArrayPickLeft) { const bool always_pick_left = true; std::valarray arr = {2.0}; - EXPECT_EQ(binarySearch(arr, 0.0), 0, always_pick_left); - EXPECT_EQ(binarySearch(arr, 100.0), 0, always_pick_left); + EXPECT_EQ(binarySearch(arr, 0.0, always_pick_left)), 0); + EXPECT_EQ(binarySearch(arr, 100.0, always_pick_left)), 0); } @@ -27,8 +27,8 @@ TEST(BinarySearchTest, ClosestMatchExact) { TEST(BinarySearchTest, ClosestMatchExactPickLeft) { const bool always_pick_left = true; std::valarray arr = {1.0, 3.0, 5.0, 7.0}; - EXPECT_EQ(binarySearch(arr, 3.0), 1, always_pick_left); - EXPECT_EQ(binarySearch(arr, 7.0), 3, always_pick_left); + EXPECT_EQ(binarySearch(arr, 3.0, always_pick_left)), 1); + EXPECT_EQ(binarySearch(arr, 7.0, always_pick_left)), 3); } TEST(BinarySearchTest, ClosestMatchInBetween) { @@ -41,9 +41,9 @@ TEST(BinarySearchTest, ClosestMatchInBetween) { TEST(BinarySearchTest, ClosestMatchInBetweenPickLeft) { const bool always_pick_left = true; std::valarray arr = {1.0, 3.0, 5.0, 7.0}; - EXPECT_EQ(binarySearch(arr, 2.1), 0, always_pick_left); - EXPECT_EQ(binarySearch(arr, 4.1), 1, always_pick_left); - EXPECT_EQ(binarySearch(arr, 6.4), 2, always_pick_left); + EXPECT_EQ(binarySearch(arr, 2.1, always_pick_left)), 0); + EXPECT_EQ(binarySearch(arr, 4.1, always_pick_left)), 1); + EXPECT_EQ(binarySearch(arr, 6.4, always_pick_left)), 2); } TEST(BinarySearchTest, ValueBeforeFirstElement) { @@ -54,7 +54,7 @@ TEST(BinarySearchTest, ValueBeforeFirstElement) { TEST(BinarySearchTest, ValueBeforeFirstElementPickLeft) { const bool always_pick_left = true; std::valarray arr = {1.0, 2.0, 3.0}; - EXPECT_EQ(binarySearch(arr, -1.0), 0, always_pick_left); + EXPECT_EQ(binarySearch(arr, -1.0, always_pick_left)), 0); } TEST(BinarySearchTest, ValueAfterLastElement) { @@ -65,7 +65,7 @@ TEST(BinarySearchTest, ValueAfterLastElement) { TEST(BinarySearchTest, ValueAfterLastElementPickLeft) { const bool always_pick_left = true; std::valarray arr = {1.0, 2.0, 3.0}; - EXPECT_EQ(binarySearch(arr, 5.0), 2, always_pick_left); + EXPECT_EQ(binarySearch(arr, 5.0, always_pick_left)), 2); } TEST(BinarySearchTest, EqualDistanceChoosesLeft) { @@ -80,7 +80,7 @@ TEST(BinarySearchTest, EqualDistanceChoosesLeftPickLeft) { std::valarray arr = {1.0, 2.0, 3.0, 4.0}; // `2` and `3` equally distant; picks left (`2`) - EXPECT_EQ(binarySearch(arr, 2.5), 1, always_pick_left); + EXPECT_EQ(binarySearch(arr, 2.5, always_pick_left)), 1); } TEST(BinarySearchTest, ThrowsOnEmptyArray) { From d6ef793c54e629bc846ed66b1ea5d6e1af35a32b Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Wed, 9 Jul 2025 20:09:18 -0700 Subject: [PATCH 14/16] fix argument position --- .../cxx/isce3/core/utilities/binarySearch.cpp | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/cxx/isce3/core/utilities/binarySearch.cpp b/tests/cxx/isce3/core/utilities/binarySearch.cpp index ef067ad2d..93f2dfea6 100644 --- a/tests/cxx/isce3/core/utilities/binarySearch.cpp +++ b/tests/cxx/isce3/core/utilities/binarySearch.cpp @@ -13,8 +13,8 @@ TEST(BinarySearchTest, SingleElementArray) { TEST(BinarySearchTest, SingleElementArrayPickLeft) { const bool always_pick_left = true; std::valarray arr = {2.0}; - EXPECT_EQ(binarySearch(arr, 0.0, always_pick_left)), 0); - EXPECT_EQ(binarySearch(arr, 100.0, always_pick_left)), 0); + EXPECT_EQ(binarySearch(arr, 0.0, always_pick_left), 0); + EXPECT_EQ(binarySearch(arr, 100.0, always_pick_left), 0); } @@ -27,8 +27,8 @@ TEST(BinarySearchTest, ClosestMatchExact) { TEST(BinarySearchTest, ClosestMatchExactPickLeft) { const bool always_pick_left = true; std::valarray arr = {1.0, 3.0, 5.0, 7.0}; - EXPECT_EQ(binarySearch(arr, 3.0, always_pick_left)), 1); - EXPECT_EQ(binarySearch(arr, 7.0, always_pick_left)), 3); + EXPECT_EQ(binarySearch(arr, 3.0, always_pick_left), 1); + EXPECT_EQ(binarySearch(arr, 7.0, always_pick_left), 3); } TEST(BinarySearchTest, ClosestMatchInBetween) { @@ -41,9 +41,9 @@ TEST(BinarySearchTest, ClosestMatchInBetween) { TEST(BinarySearchTest, ClosestMatchInBetweenPickLeft) { const bool always_pick_left = true; std::valarray arr = {1.0, 3.0, 5.0, 7.0}; - EXPECT_EQ(binarySearch(arr, 2.1, always_pick_left)), 0); - EXPECT_EQ(binarySearch(arr, 4.1, always_pick_left)), 1); - EXPECT_EQ(binarySearch(arr, 6.4, always_pick_left)), 2); + EXPECT_EQ(binarySearch(arr, 2.1, always_pick_left), 0); + EXPECT_EQ(binarySearch(arr, 4.1, always_pick_left), 1); + EXPECT_EQ(binarySearch(arr, 6.4, always_pick_left), 2); } TEST(BinarySearchTest, ValueBeforeFirstElement) { @@ -54,7 +54,7 @@ TEST(BinarySearchTest, ValueBeforeFirstElement) { TEST(BinarySearchTest, ValueBeforeFirstElementPickLeft) { const bool always_pick_left = true; std::valarray arr = {1.0, 2.0, 3.0}; - EXPECT_EQ(binarySearch(arr, -1.0, always_pick_left)), 0); + EXPECT_EQ(binarySearch(arr, -1.0, always_pick_left), 0); } TEST(BinarySearchTest, ValueAfterLastElement) { @@ -65,7 +65,7 @@ TEST(BinarySearchTest, ValueAfterLastElement) { TEST(BinarySearchTest, ValueAfterLastElementPickLeft) { const bool always_pick_left = true; std::valarray arr = {1.0, 2.0, 3.0}; - EXPECT_EQ(binarySearch(arr, 5.0, always_pick_left)), 2); + EXPECT_EQ(binarySearch(arr, 5.0, always_pick_left), 2); } TEST(BinarySearchTest, EqualDistanceChoosesLeft) { @@ -80,7 +80,7 @@ TEST(BinarySearchTest, EqualDistanceChoosesLeftPickLeft) { std::valarray arr = {1.0, 2.0, 3.0, 4.0}; // `2` and `3` equally distant; picks left (`2`) - EXPECT_EQ(binarySearch(arr, 2.5, always_pick_left)), 1); + EXPECT_EQ(binarySearch(arr, 2.5, always_pick_left), 1); } TEST(BinarySearchTest, ThrowsOnEmptyArray) { From 5f3c682d08b9e8ad54402a4671137419b9ac68d6 Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Wed, 9 Jul 2025 23:08:29 -0700 Subject: [PATCH 15/16] handle values outside array; improve docstrings; fix logic for array[right] == value --- cxx/isce3/core/Utilities.h | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/cxx/isce3/core/Utilities.h b/cxx/isce3/core/Utilities.h index 0fca46e99..d02b94f9a 100644 --- a/cxx/isce3/core/Utilities.h +++ b/cxx/isce3/core/Utilities.h @@ -337,6 +337,13 @@ namespace isce3 { namespace core { /** Searches array for index closest to provided value * * Assumes the array is sorted in ascending order and contains no NaNs. + * @param array The input array, which must be sorted in ascending order and contain no NaNs. + * @param value The value to search for. + * @param always_pick_left If true, and `value` is strictly between two array elements, + * the function will return the lower index (left). If false, the + * function returns the closest index (left or right). + * + * @return The index of the array element closest to `value`. */ inline int binarySearch(const std::valarray & array, double value, const bool always_pick_left = false) { @@ -348,16 +355,24 @@ namespace isce3 { namespace core { return 0; } - // Do the binary search int left = 0; int right = array.size() - 1; + // Test extreme values + if (value <= array[left]) { + return left; + } + if (array[right] <= value) { + return right; + } + std::size_t count = 0; // `max_iter` provides a safeguard against unexpected behavior. // The loop is guaranteed to converge with correct input, but we include // this check as a safe measure for unexpected edge cases. - std::size_t max_iter = std::ceil(std::log2(array.size())) + 1; + constexpr std::size_t padding = 2; + std::size_t max_iter = std::ceil(std::log2(array.size())) + padding; while (left + 1 < right) { const int middle = left + (right - left) / 2; const auto middle_value = array[middle]; @@ -375,14 +390,16 @@ namespace isce3 { namespace core { } } - if (always_pick_left) { - return left; - } - + // Test for NaNs if (std::isnan(array[left]) || std::isnan(array[right])) { throw std::invalid_argument("input array may not contain NaN values"); } + // If always pick left and the `right` index is not pointing at value + if (always_pick_left && value < array[right]) { + return left; + } + // Return the closest of left and right if (std::abs(array[left] - value) <= std::abs(array[right] - value)) { return left; From f314eaa7af19ce1d0edb915215bcdbdad782bc91 Mon Sep 17 00:00:00 2001 From: "Gustavo H. X. Shiroma" Date: Wed, 9 Jul 2025 23:11:48 -0700 Subject: [PATCH 16/16] improve docstrings --- cxx/isce3/core/Utilities.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cxx/isce3/core/Utilities.h b/cxx/isce3/core/Utilities.h index d02b94f9a..1cc96bd0e 100644 --- a/cxx/isce3/core/Utilities.h +++ b/cxx/isce3/core/Utilities.h @@ -337,11 +337,13 @@ namespace isce3 { namespace core { /** Searches array for index closest to provided value * * Assumes the array is sorted in ascending order and contains no NaNs. - * @param array The input array, which must be sorted in ascending order and contain no NaNs. + * + * @param array The input array, which must be sorted in ascending order + * and contain no NaNs. * @param value The value to search for. - * @param always_pick_left If true, and `value` is strictly between two array elements, - * the function will return the lower index (left). If false, the - * function returns the closest index (left or right). + * @param always_pick_left If true, and `value` is strictly between two + * array elements, the function will return the lower index (left). + * If false, the function returns the closest index (left or right). * * @return The index of the array element closest to `value`. */