sequence() should not convert the index to T

[bjude]

thrust::sequence currently uses the following struct to compute the sequence value (found in thrust/system/detail/generic/sequence.inl)

template <typename T>
struct compute_sequence_value
{
  T init;
  T step;

  __thrust_exec_check_disable__
  __host__ __device__
  T operator()(std::size_t i) const
  {
    return init + step * static_cast<T>(i);
  }
};
}

the static_cast<T>(i) will cause the algorithm to fail to compile in cases where step * i would otherwise work just fine (in my case T is float3)

Is this an intended limitation of sequence()? It would make more sense to me to use step * i if that was valid, falling back to trying the static_cast only if that would fail.

[alliepiper]

The cast was added to address conversion warnings, but we should fix this by specializing the compute_sequence_value to only do the cast when T is a built-in numeric type.

In the meantime, you can use thrust::tabulate directly as a workaround.

[alliepiper]

I don't think we can do this:

cases where step * i would otherwise work just fine (in my case T is float3)

This will fail to compile, even without the cast: https://www.godbolt.org/z/88EEhdbbE

int main()
{
  float3 init{1.f, 1.f, 1.f};
  float3 step{0.5f, 0.5f, 0.5f};
  std::size_t i = 3;
  
  float3 result = init + step * i;
}

error: no operator "*" matches these operands
            operand types are: float3 * std::size_t

I'm closing this with the suggestion to use thrust::tabulate instead. Feel free to reopen if there's something I'm missing.

[bjude]

I have cuda_helpers.h included which defined the arithmetic (scalar-vector and vector-vector) ops so its more like this: https://www.godbolt.org/z/3cadPMrde

Given that the documentation lists the operation being performed as init + step * (i - first), either that should be the operation or the documentation needs to be updated indicating that there needs to be a conversion from std::size_t to T available.
IMO the former makes more sense. If an overload/specialisation is desired in order to silence spurious warnings then thats fine, but its an odd limitation to have for this algorithm to require a conversion

[bjude]

@allisonvacanti FYI i cant re-open the issue because a project contributor closed it

[alliepiper]

Gotcha. I'll reopen this and take another look at some point.