Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix radix sort #431

Open
faheel opened this issue Jun 5, 2022 · 2 comments
Open

Fix radix sort #431

faheel opened this issue Jun 5, 2022 · 2 comments

Comments

@faheel
Copy link
Member

faheel commented Jun 5, 2022

Radix sort seems to be broken as pointed out in #430

@pm100
Copy link

pm100 commented Apr 4, 2023

there are two issues here.

  • there is a trivial bug to do with passing arguments in the wrong order. I can post a fix for that. Its a bug in the sort not in the test harness
  • radix sort cannot work with negative values as currently coded. This causes the test harness to fail even with the first thing fixed.

I can post the fix to the first one, this makes code like this work

vector<int> arr{ 1, 8, 5, 12, 3 };
radix_sort(arr, 1, true);
return 0;

but crashes with (negative array indexes get generated by get_digit)

vector<int> arr{ 1, 8, -5, 12, 3 };
radix_sort(arr, 1, true);
return 0;

the second one is tougher. Two choices

  • fix it so that it can support negative values. Not sure if radix sort can be made to do that
  • doc it as an algorithm that only supports >=0 values and enforce that. Test harness would need to be changed too

I have verified that if I change the test harness to only generate >=0 values it works fine

@pm100
Copy link

pm100 commented Apr 4, 2023

one way to fix would be

  • partition into neg and pos sub vectors
  • change signs of neg part
  • sort pos one way, neg the other way
  • merge

I have coded this up, works fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants