Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

thrust::find doesn't handle asymmetric equality operator #1229

Closed
dkolsen-pgi opened this issue Jul 11, 2020 · 7 comments
Closed

thrust::find doesn't handle asymmetric equality operator #1229

dkolsen-pgi opened this issue Jul 11, 2020 · 7 comments
Assignees
Milestone

Comments

@dkolsen-pgi
Copy link
Collaborator

thrust::find fails to compile when the input iterator's value type is different from the type of the value being searched for and neither type is convertible to the other, even when there is an overloaded operator== that takes objects of those two different types.

Given the following CUDA program:

#include <thrust/host_vector.h>
#include <thrust/device_vector.h>
#include <thrust/find.h>
#include <cassert>

class Weird {
  int value;
 public:
  friend __host__ __device__ bool operator==(int x, Weird y) {
    return x == y.value;
  }
  __host__ __device__ Weird(int val, int) : value(val) { }
};

int main() {
  thrust::host_vector<int> v;
  for (int i = 0; i < 10000; ++i) {
    v.push_back(i);
  }
  thrust::device_vector<int> dv(v);
  auto result = thrust::find(dv.begin(), dv.end(), Weird(333, 0));
  assert(*result == 333);
}

When compiled with NVCC 11.0 and the latest Thrust source from the main branch, this fails with:

/proj/cuda/thrust/master/thrust/detail/functional/operators/operator_adaptors.h(108): error: function "thrust::equal_to<T>::operator() [with T=int]" cannot be ca
lled with the given argument list
            argument types are: (int, Weird)
            object type is: thrust::equal_to<int>

(followed by a typical template instantiation novel).

This is a recent regression. It broke somewhere between July 3 and July 6. I have not looked to see which commit might have caused the regression. When compiled with the Thrust in CUDA 11.0, this test case compiles successfully.

@alliepiper
Copy link
Collaborator

It is likely related to 0b48df1.

It looks like adding a equal_to<void> specialization (like in thrust/functional.h in b528451) should fix this.

@alliepiper alliepiper self-assigned this Jul 13, 2020
@alliepiper alliepiper added this to the 1.10.0 milestone Jul 13, 2020
@dkolsen-pgi dkolsen-pgi modified the milestones: 1.10.0, 1.9.10-1 Jul 13, 2020
@dkolsen-pgi
Copy link
Collaborator Author

I just realized that the regression is in the 1.9.10-1 branch, not just in main. This is a regression in our soon-to-be-code-frozen release. Please get the fix into 1.9.10-1 ASAP.

@alliepiper
Copy link
Collaborator

If it's what I think it is, we can revert the change in the staging branch ASAP. We should be able to just revert the commit that was likely to introduce this.

If you get a chance to test this out tonight let me know, but otherwise I'll test it out and integrate it tomorrow.

In your thrust git repo, checkout the staging branch and revert f483906.

git checkout staging/1.9.10-1
git revert f483906

alliepiper added a commit that referenced this issue Jul 14, 2020
This reverts commit f483906.

See discussion in #1229.

Bug 3043659
@alliepiper
Copy link
Collaborator

Confirmed that reverting f483906 fixes the regression. Fixed in perforce CL 28805582 and staging/1.9.10-1 commit c3c368a.

Let's leave this issue open until we properly address this in main.

@alliepiper alliepiper modified the milestones: 1.9.10-1, 1.10.0 Jul 14, 2020
alliepiper added a commit to alliepiper/thrust that referenced this issue Jul 16, 2020
alliepiper added a commit to alliepiper/thrust that referenced this issue Jul 17, 2020
alliepiper added a commit to alliepiper/thrust that referenced this issue Jul 17, 2020
alliepiper added a commit to alliepiper/thrust that referenced this issue Jul 17, 2020
alliepiper added a commit to alliepiper/thrust that referenced this issue Jul 24, 2020
alliepiper added a commit to alliepiper/thrust that referenced this issue Jul 27, 2020
@bjude
Copy link
Contributor

bjude commented Aug 2, 2020

this issue is also present with thrust::equal, which delegates down to thrust::equal_to<T>, not sure if it's resolved by the commits Allison referenced

@alliepiper
Copy link
Collaborator

@bjude All issues from using heterogeneous types in the placeholder code will be fixed by that patch. If you have a small reproduction of the case you encountered, please share it and I'll verify/add a regression test for it.

alliepiper added a commit to alliepiper/thrust that referenced this issue Aug 3, 2020
alliepiper added a commit to alliepiper/thrust that referenced this issue Aug 3, 2020
alliepiper added a commit to alliepiper/thrust that referenced this issue Aug 7, 2020
alliepiper added a commit to alliepiper/thrust that referenced this issue Aug 28, 2020
alliepiper added a commit that referenced this issue Aug 28, 2020
@alliepiper
Copy link
Collaborator

Fixed in #1237.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants