Problem with Inversion Count Algorithm Implementation

Question

Problem Statement

I am attempting to solve this problem.

Short summary of the problem: Given an unsorted array, find the total number of inversion pairs. Definition of inversion: index i and j forms a valid inversion pair if i < j && arr[i] > arr[j]

My approach:

1. I attempted to solve this with Divide and Conque. I divided the array into two parts and recursively solve each of them.
2. After the recursive call is completed, each half should be sorted and should return the number of the inversion count.
   The answer for the current array will be the sum of answers from both halves + the number of inversion pairs between both halves.
   See this figure for a better understanding of this step:

Photo Credit: geeks for geeks

3. To find the number of crossing inversions, I looped over the first array and for each element, I did a binary search in the second half to get the location of the exact element or the location of the first bigger element(more about this here).

While this approach seems correct to me and passes few test cases, it gives a wrong answer to some hidden test cases.

I need help understanding if there is any flaw in my algorithm or implementation.

My implementation:

    static long inversionCount(long arr[], long N){
        return solve(0, (int)N-1, arr);
    }
    
    static long solve(int l, int r, long[] arr){
        if(l >= r) return 0;
        
        int mid = l+(r-l)/2;
        
        long countL = solve(l, mid, arr); //solve left half
        long countR = solve(mid+1, r, arr); //solve right half
        
        long countM = 0;
        
        //count crossing inversions
        for(int idx = l; idx <= mid; idx++){
            int position = Arrays.binarySearch(arr, mid+1, r+1, arr[idx]);
            if(position<0) position = -(position+1);
            
            countM += position-mid-1;
        }
        
        Arrays.sort(arr, l, r);
        
        return countM+countL+countR;
    }

Answer 1

I need help understanding if there is any flaw in my algorithm or implementation.

There are at least two flaws in your implementation. Here is a test application that helps you to find a failing test case.

public class InversionCountTest {
  /**
   * Fixed array length to be used for the test cases.
   *
   * <p>A failing test case with a short array is simpler to understand than a
   * failing test case with a long array, so check small array lengths first.
   */
  static final int ARR_LENGTH = 4;

  /**
   * Maximum long value to be used as array element.
   *
   * <p>Note that a smaller value of {@link #MAX_VALUE} increases the chance of
   * duplicate entries. Should {@link InversionCount#inversionCount} work for
   * duplicate entries? Then we should test those cases.
   */
  static final long MAX_VALUE = 10L;

  /** Random-number seed for the test-case generator. */
  static final long SEED = 42L;

  /** Number of randomized test cases. */
  static final int NUMBER_OF_RANDOMIZED_TEST_CASES = 100_000;

  public static void main(String[] args) {
    Random rnd = new Random(SEED);

    LongSupplier getNextLong = () -> {
      long randomLongValue = rnd.nextLong() + rnd.nextLong();
      long minValue = 1L;
      long range = MAX_VALUE - minValue;
      if (range == 0L) {
        return minValue;
      }
      if (range < 0L) {
        throw new AssertionError("invalid range");
      }
      return minValue + Math.abs(randomLongValue % range);
    };

    int testCaseCount = NUMBER_OF_RANDOMIZED_TEST_CASES;
    for (int testCaseIdx = 0; testCaseIdx < testCaseCount; ++testCaseIdx) {
      long[] arr = LongStream.generate(getNextLong).limit(ARR_LENGTH).toArray();

      long reference = inversionCountReference(arr, arr.length);
      long computed = InversionCount.inversionCount(arr, arr.length);
      if (reference != computed) {
        System.out.println("failing test case: " + Arrays.toString(arr));
        System.out.println("  expected: " + reference);
        System.out.println("  actual: " + computed);
      }
    }
  }

  static long inversionCountReference(long arr[], int N) {
    long inversionCount = 0L;
    for (int i = 0; i < N; ++i) {
      for (int j = i + 1; j < N; ++j) {
        // TODO: implement this
        throw new UnsupportedOperationException("not implemented yet");
      }
    }
    return inversionCount;
  }
}

Hints:

• One flaw in your implementation is an off-by-one error. Usually, ranges are given by an inclusive lower index and an exclusive upper index. You introduced a bug by breaking this convention (carefully check variable r).
• It might help to re-consider the recursion break: How do you compute the inversion count of an array of length one? Can this simplify your recursion?