Fix index difference to follow the pandas format

[amiralimi]

Description

This PR fixes an error in Index.difference where the function keeps duplicate elements while pandas removes the duplicates. The tests had no inputs with duplicates, so I added new tests too (I added the test from the original issue).

• closes [BUG] Index.difference does not uniquify output for duplicate indexes #14489

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[vyasr]

/ok to test

[vyasr]

@wence- could you have a look at this when you get a chance? Thanks!

[shwina]

Should we also call unique() on the right hand side to make the merge smaller?

[amiralimi]

I tried this with a few test cases, in some it had a better performance, and in some, it was worse. But pandas does this.

[shwina]

OK, I'd just add a note here to alert future readers about potential performance issues:

# NOTE: may need to investigate calling `unique()` on the LHS before the merge for better performance 

[wence-]

I am somewhat surprised that calling unique on the right column sometimes improves performance. unique calls stable_distinct which builds a hash table to uniquify things.

The leftanti join builds a hash table for the right column and then probes that hash table with the left column to return those rows in the left column that are not in the hash table.

So calling unique() on the right column would just seem to be an extra hash-table build for no gain.

Can you show the test cases you ran to check performance @amiralimi ?

[shwina]

@amiralimi - this is looking good. Could you run the style check, resolve any style issues found, and push a new commit with the updated style? For this, using pre-commit is helpful:

pre-commit install
pre-commit run --all

[amiralimi]

@shwina Thanks for helping out. I just ran pre-commit. This is my first time contributing to an open-source project, is there anything I need to do?

[shwina]

/ok to test

[shwina]

/ok to test

[shwina]

/ok to test

[wence-]

Thanks, this looks good to me! It would be nice to see the small performance tests you ran to see if uniquifying the right column was helpful.

[wence-]

I am somewhat surprised that calling unique on the right column sometimes improves performance. unique calls stable_distinct which builds a hash table to uniquify things.

The leftanti join builds a hash table for the right column and then probes that hash table with the left column to return those rows in the left column that are not in the hash table.

So calling unique() on the right column would just seem to be an extra hash-table build for no gain.

Can you show the test cases you ran to check performance @amiralimi ?

[amiralimi]

Hi @wence- .
You are correct. I just reran the tests and calling unique on RHS always has worse performance.
This is the test I wrote:

import cudf
import cupy
import time

l1 = cudf.Index(cupy.random.randint(0, 100, 10000000))
r1 = cudf.Index(cupy.random.randint(0, 100, 1000000))

r2 = cudf.Index(cupy.random.randint(0, 1000000, 1000000))
l2 = cudf.Index(cupy.random.randint(0, 1000, 10000000))

l3 = cudf.Index(cupy.random.randint(0, 1000000, 10000000))
r3 = cudf.Index(cupy.random.randint(0, 1000000, 10000000))

l4 = cudf.Index(cupy.random.randint(0, 1000, 10000000))
r4 = cudf.Index(cupy.random.randint(0, 1000000, 10000000))

start = time.time()
l1.difference(r1)
end = time.time()
print(f"test 1: {end - start}")

start = time.time()
l2.difference(r2)
end = time.time()
print(f"test 2: {end - start}")

start = time.time()
l3.difference(r3)
end = time.time()
print(f"test 3: {end - start}")

start = time.time()
l4.difference(r4)
end = time.time()
print(f"test 4: {end - start}")

and this is the output:

// no RHS unique
test 1: 0.04081010818481445
test 2: 0.044335126876831055
test 3: 0.04413151741027832
test 4: 0.01869058609008789

// RHS unique
test 1: 0.04834389686584473
test 2: 0.057317495346069336
test 3: 0.06256484985351562
test 4: 0.03918814659118652

[shwina]

/merge

[shwina]

Thanks @amiralimi - we're honored to have you contribute to cuDF as your first open-source contribution! Hope we'll see more!

[amiralimi]

Thanks @shwina . I really enjoyed working on CuDF and will try to contribute more to it.

[wence-]

Thanks!