fix(sorts): prevent comb_sort from returning an unsorted list - #15003
Open
SEPURI-SAI-KRISHNA wants to merge 1 commit into
Open
fix(sorts): prevent comb_sort from returning an unsorted list#15003SEPURI-SAI-KRISHNA wants to merge 1 commit into
SEPURI-SAI-KRISHNA wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe your change:
comb_sortcan return an unsorted list.The gap is updated with
gap = int(gap / shrink_factor)and the loop is terminatedby
if gap <= 1: completed = True. Once the gap reaches0, the inner pass runsdata[index] > data[index + 0], i.e. it compares every element with itself. Thatcomparison is never true, so the pass can never reset
completedback toFalse,and the function returns while the list is still unsorted.
Comb sort requires the final gap-1 passes to repeat until a pass makes no swaps.
Because the gap is allowed to hit
0, that never happens.Reproducer on current
master:This is not a rare edge case: 240 of the 5040 permutations of
range(7)(~5%)come back unsorted, and the failure rate stays in that range for larger lists.
The existing doctests all happen to use inputs that finish sorting before the gap
reaches
0, which is why this went unnoticed.Fix
Clamp the gap so it never drops below
1:The gap-1 passes now repeat until one of them makes no swaps, which is the actual
termination condition of comb sort.
gap <= 1becomesgap == 1since the gap canno longer be
0. Empty and single-element lists still terminate immediately, sincethe inner
while index + gap < len(data)condition is false on the first check.I also added
comb_sort([2, 0, 3, 4, 5, 6, 1])as a regression doctest — it failson
masterand passes with the fix. It is the test for this bug rather than anindependent doctest change.
Verification
All permutations of
range(n)fornin0..8sort correctly (every one of themfails-or-passes checked against
sorted()); onmasterthis fails fromn = 7.50,000 randomised lists (lengths 0-60, values -100..100, duplicates included) all
match
sorted().Lists of strings still sort correctly.
ruff check,ruff format --check,mypy --ignore-missing-importsandpytest --doctest-modules sorts/comb_sort.pyall 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: