Skip to content

fix(searches): correct ternary search pivots and exclusive bounds - #15005

Open
SEPURI-SAI-KRISHNA wants to merge 1 commit into
TheAlgorithms:masterfrom
SEPURI-SAI-KRISHNA:fix/ternary-search-out-of-range-pivots
Open

fix(searches): correct ternary search pivots and exclusive bounds#15005
SEPURI-SAI-KRISHNA wants to merge 1 commit into
TheAlgorithms:masterfrom
SEPURI-SAI-KRISHNA:fix/ternary-search-out-of-range-pivots

Conversation

@SEPURI-SAI-KRISHNA

Copy link
Copy Markdown

Describe your change:

ite_ternary_search and rec_ternary_search raise IndexError on most non-trivial
inputs, and return -1 for elements that are present.

>>> from searches.ternary_search import ite_ternary_search
>>> ite_ternary_search(list(range(200)), 150)
Traceback (most recent call last):
  ...
IndexError: list index out of range

There are two independent defects.

1. The pivots are not offsets into the current search window.

one_third = (left + right) // 3 + 1
two_third = 2 * (left + right) // 3 + 1

These are derived from left + right, so they are not positions inside
array[left:right]. As soon as left grows, they run past the end of the array.
For example with left = 100, right = 115 the second pivot evaluates to 144,
well outside the window and outside a 115-element array. Roughly 46% of searches
on arrays of 30-300 elements crash with IndexError
.

2. The narrowing steps drop one element per step.

right is an exclusive bound everywhere else in this file — it is initialised to
len(array), and lin_search iterates range(left, right). But the narrowing used
right = one_third - 1 and right = two_third - 1, which discards the element at
the new right index. Since the pivot itself has already been compared and ruled
out, only right = one_third / right = two_third is correct. This silently loses
a candidate on every iteration, so present values report as not found:

>>> data = [0, 5, 5, 5, 7, 9, 10, 12, 13, 15, 15, 15]
>>> ite_ternary_search(data, 7)
-1                     # expected 4

On arrays of 10-30 elements about 6.6% of lookups for a value that is present
return -1
.

Why the existing doctests never caught this. Every current doctest uses an array
shorter than precision = 10, so right - left < precision is true immediately and
the function returns from lin_search on the first iteration. The ternary logic was
never executed by any test.

Fix

Compute both pivots as offsets inside the half-open window array[left:right], and
narrow with the exclusive bound the rest of the file already uses:

third = (right - left) // 3
one_third = left + third
two_third = left + 2 * third
...
elif target < array[one_third]:
    right = one_third
elif array[two_third] < target:
    left = two_third + 1
else:
    left = one_third + 1
    right = two_third

Both pivots are now guaranteed to satisfy left <= one_third <= two_third < right,
and every branch strictly shrinks the window, so the search always terminates.

The loop guard also becomes while left < right, matching the half-open range (with
left == right the window is empty).

Two further small corrections in the same file:

  • lin_search's docstring now states that left is inclusive and right is
    exclusive, which is what the implementation has always done.
  • The __main__ block called rec_ternary_search(0, len(collection) - 1, ...),
    which excluded the last element of the user's input, so searching for the largest
    value printed "Not found". Corrected to len(collection).

No behaviour that the existing doctests rely on has changed — all of them still pass
unmodified.

Verification

  • 20,000 randomised sorted arrays (lengths 0-400, duplicates included), each searched
    for both a present and a possibly-absent value, against both the iterative and the
    recursive variant: 0 failures. On master the same run produces thousands of
    IndexErrors and false -1s.

  • ite_ternary_search(list(range(200)), 150) now returns 150.

  • Arrays with heavy duplicates return a valid index of the target; string arrays still
    work.

  • Added doctests that exercise the ternary path itself (a 100-element list, searched
    for every member plus one absent value) — these fail on master and pass here.

  • ruff check, ruff format --check, mypy --ignore-missing-imports,
    pytest --doctest-modules searches/ternary_search.py and pre-commit run all pass.

  • Add an algorithm?

  • Fix a bug or typo in an existing algorithm?

  • Add or change doctests? -- Note: Please avoid changing both code and tests in a single pull request.

  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the description above includes the issue number(s) with a closing keyword: "Fixes #ISSUE-NUMBER".

@algorithms-keeper algorithms-keeper Bot added enhancement This PR modified some existing files awaiting reviews This PR is ready to be reviewed labels Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting reviews This PR is ready to be reviewed enhancement This PR modified some existing files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant