Skip to content
New issue

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

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

Already on GitHub? # 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

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

No branches or pull requests

2 participants