Skip to content

Conversation

Vuenc
Copy link
Contributor

@Vuenc Vuenc commented Mar 16, 2023

Fixed the error described in issue #163
I implemented the lenient approach from my reply there (error only on if some batch instance is present in batch_x, but missing from batch_y). I implemented this as-is for the CPU version, and an equivalent check on the ptr_x/ptr_y for the GPU version to catch the case where one of batch_x/batch_y is None.

I also checked if any of the other methods suffer from the same problem, but that's not the case. Only knn and radius take batch_x,batch_y arguments, and both seem to correctly handle empty instances (don't return any edge within an empty instance).

Additionally I implemented a check in nearest() if the batch_x, batch_y are sorted (as it's a general requirement of the method, and I also rely on it when calling torch.unique_consecutive()).

@codecov-commenter
Copy link

codecov-commenter commented Mar 16, 2023

Codecov Report

Merging #168 (4c3ce21) into master (82e9df9) will increase coverage by 0.05%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@ Coverage Diff @@ ## master #168 +/- ## ========================================== + Coverage 98.83% 98.88% +0.05%  ========================================== Files 10 10 Lines 86 90 +4 ========================================== + Hits 85 89 +4  Misses 1 1 
Impacted Files Coverage Δ
torch_cluster/nearest.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Vuenc Vuenc force-pushed the nearest-empty-batch-instances branch from 2818a01 to 14271b2 Compare March 16, 2023 11:02
Copy link
Owner

@rusty1s rusty1s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@rusty1s rusty1s merged commit 0e10871 into rusty1s:master Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants