nth_element implementation based on modified quick_sort, not working as expected

Question

For the purpose of understanding quick_sort I am trying to implement nth_element.
As ground truth for testing I am using std::nth_element
My algorithm fails on many inputs, for ex on a = {6,1,7,5,3,8,2,4,9}
How to fix it?

std::random_device dev;
std::mt19937 rng(dev());

int kth_element(vector<long long> a, int l, int r, int k) {
  if(l >= r) return a[l];
  int i = l, j = r;
  uniform_int_distribution<int> dist(l,r);
  int X = a[dist(rng)];
  while(i < j) {
    while(a[i] < X) ++i;
    while(a[j] > X) --j;
    if(i < j) {
      swap(a[i], a[j]);
      ++i;
      --j;
    }
  }
  if(k >= l && k < i)
    return kth_element(a, l,i-1,k);
  else if (j < k && k <= r)
    return kth_element(a,j+1,r,k);
  return X;
}

int kth_element(vector<long long> a, int k) {
  int n = a.size();
  return kth_element(a,0,n-1,k);
}

int main(int argc, char* argv[]) {
  vector<long long> a = {1,2,3,4,5,6,7,8,9};
  int n = a.size();
  for(int i = 0; i < n; ++i)
    cout << kth_element(a,i) << '\n';

  random_device rd;
  mt19937 rng(rd());
  shuffle(a.begin(), a.end(), rng);
  show_vars(a);
  for(int i = 0; i < n; ++i) {
    cout << i << ": " << kth_element(a,i);
    nth_element(a.begin(), a.begin()+i,a.end());
    cout << ", " << a[i] << '\n';
    assert(kth_element(a,i) == a[i]);
  }

  return 0;
}

Update if I do testing in the loop, the algorithm fails:

int main(int argc, char* argv[]) {
  vector<long long> a = {1,2,3,4,5,6,7,8,9};
  int n = a.size();
  // for(int i = 0; i < n; ++i)
  //   cout << kth_element(a,i) << '\n';

  random_device rd;
  mt19937 rng(rd());
  for(int t = 0; t < 1000; ++t) {
    shuffle(a.begin(), a.end(), rng);
    // show_vars(a);
    for(int i = 0; i < n; ++i) {
      // show_vars(i);
      long long kth = kth_element(a,i);
      cout << i << ": " << kth;
      nth_element(a.begin(), a.begin()+i,a.end());
      cout << ", " << a[i] << '\n';
      // show_vars(kth, a[i]);
      assert(kth == a[i]);
    }
  }

  return 0;
}

@john missed the wrapper function, it is fixed now, thank you — spin_eight, May 13 '20 at 10:15
You forgot to return both inside if and else if when calling for `kth_element`. — srt1104, May 13 '20 at 10:34
@lucieon Thank you very much! That's it! Added return statements, but it still fails if I do shuffle in the loop, I added the code for that — spin_eight, May 13 '20 at 10:36
@n.'pronouns'm. I appended it to the end of the question, after "Update if I do testing in the loop, the algorithm fails:" — spin_eight, May 13 '20 at 10:53
Your partitioning algorithm is wrong, I suggest extracting it to a function and testing it separately. — n. 'pronouns' m., May 13 '20 at 11:34

score 0 · Accepted Answer · answered May 13 '20 at 21:36

In case if anybody tries to implement nth_element based on Hoar partition:

use 2 pointers i,j and maintain loop invariant:
for k = 0..i-1 a[k] <= X and for z = j+1 .. n-1 a[z] >= X
after the loop, the array might be partitioned in 2 ways:
...| j | i |...
or ...| j | X | i |... all elements(left part) with idx in [L,j] <= X
with idx(right part) in [i,R] >= X
based on the number of elements in left/right part recur on the required path

std::random_device dev;
std::mt19937 rng(dev());
long long kth(vector<long long>& a, int L, int R, int k) {
  if(L>=R) return a[L];

  int i = L,j = R;

  std::uniform_int_distribution<std::mt19937::result_type> dist(L,R);
  long long X = a[dist(rng)];

  while(i <= j) {
    while(a[i] < X) ++i;
    while(a[j] > X) --j;
    if(i <= j) {
      swap(a[i], a[j]);
      ++i;
      --j;
    }
  }


  if(k <= j)
    return kth(a, L,j,k);
  if (k >=i)
    return kth(a,i,R,k);
  return X;
}

This doesn't look like an answer to me. The answer section is for answers. — n. 'pronouns' m., May 15 '20 at 08:25

nth_element implementation based on modified quick_sort, not working as expected

1 Answers1