variadic overload of make_zip_iterator

[andrewcorrigan]

Would it make sense to add a variadic overload of make_zip_iterator that composes the existing make_zip_iterator with make_tuple? I have this in my own code, and I find that it reduces syntactic overhead.

template<typename... Iterators>
__host__ __device__
  zip_iterator<thrust::tuple<Iterators...>> make_zip_iterator(thrust::tuple<Iterators...> t)
{
    return zip_iterator<thrust::tuple<Iterators...>>(t);
}


template<typename... Iterators>
__host__ __device__
  zip_iterator<thrust::tuple<Iterators...>> make_zip_iterator(Iterators... its)
{
    return make_zip_iterator(thrust::make_tuple(its...));
}

To disambiguate the single argument overloads, I replaced the existing overload for IteratorTuple with the one above that accepts thrust::tuple<Iterators...>.

[andrewcorrigan]

@jaredhoberock Will you entertain a pull request on this one too?

[jaredhoberock]

This needs to be merged at the same time as variadic tuple.

[schiller-manuel]

@jaredhoberock This already works with the current, non variadic tuple implementation (if sizeof...(Iterators) <= 10)

[jaredhoberock]

That may be, but I'd prefer to just keep it simple and introduce a variadic overload of this function with variadic tuple.

[bjude]

Is this something that could be implemented once NVIDIA/cccl#742 is implemented?

[alliepiper]

This should be doable without much trouble now that we have C++11 or greater as a hard requirement.

It's not a high priority issue right now, but I'm happy to review and test a PR if someone wants to work on it.

[bjude]

Does @jaredhoberock 's concern about holding off until we have a variadic thrust::tuple still hold?

The implementation of the variadic make_zip_iterator wouldnt need to change once the variadic tuple is implemented so i dont see a reason not to implement it now

[alliepiper]

From my perspective, I don't think it's worth waiting on tuple.

The tentative plan regarding tuple is to switch thrust::tuple to an alias of cuda::std::tuple once it's available in libcu++. Right now, adding tuple to libcu++ has stalled due to compatibility issues on MSVC (see NVIDIA/libcudacxx#17), and I'm not convinced that it'll be a drop-in replacement that we can easily migrate to in Thrust without some refactoring. I'm not sure when we'll be able to prioritize the tuple update/integration.

There are some additional obstacles to using libcu++ in Thrust due to Thrust's role in the NV HPC SDK. Those are several months away from being resolved.

Since the tuple update is pretty far down the roadmap and variadic make_zip_iterator is an orthogonal QOL improvement, I'd rather not block an actionable modernization on something that's currently a wishlist item.

That said, I'm okay with pushing this back if there's an angle that I'm not considering.

[andrewcorrigan]

The tentative plan regarding tuple is to switch thrust::tuple to an alias of cuda::std::tuple once it's available in libcu++. Right now, adding tuple to libcu++ has stalled due to compatibility issues on MSVC (see NVIDIA/libcudacxx#17), and I'm not convinced that it'll be a drop-in replacement that we can easily migrate to in Thrust without some refactoring. I'm not sure when we'll be able to prioritize the tuple update/integration.

Sorry for going a bit off-topic, but I wanted to add my two cents here:

In terms of a new variadic tuple not being a drop-in replacement in Thrust, I agree, but my experience was that switching to a variadic tuple implementation was mostly just tedious work. When I updated Thrust circa 2015 to support variadic tuple the bulk of the work was getting rid of detail::cons, null_type, and converting all of the specializations to tuples of length 1-10 to a single variadic definition. Back then I was completely new to C++11 so it was very challenging for me, but in hindsight I think it ought to be a straightforward task that wouldn't take that much time.

[alliepiper]

@andrewcorrigan That's good to know. If we end up needing to preserve the existing thrust::tuple this sounds straightforward enough.

But we should find out whether or not the libcu++ tuple will work out before anyone invests too much energy into updating the current implementation. If we're able to switch cleanly, our team will only have to maintain a single tuple implementation, and then Thrust can offload testing / maintenance to libc++/libcu++. Any work done on the current tuple in the meantime would get thrown out if our plans succeed.

Roadmap-wise, this is likely to be a pattern going forward -- many of the pre-C++11 metaprogramming utilities in Thrust (and CUB) can be replaced with the more rigorously tested implementations in libcu++, and we want to leverage code reuse here as much as possible as these projects evolve.

[andrewcorrigan]

Makes sense. To clarify, I was actually referring to updating the rest of Thrust to deal with variadic tuples, regardless of which tuple implementation you end up using. Much of the work is entirely independent of whether libcu++ tuple is adopted as opposed to updating the tuple implementation embedded in Thrust. For example, code like this:

// specialize is_unwrappable
// a tuple is_unwrappable if any of its elements is_unwrappable
template<
  typename T0, typename T1, typename T2,
  typename T3, typename T4, typename T5,
  typename T6, typename T7, typename T8,
  typename T9
>
  struct is_unwrappable<
    thrust::tuple<T0,T1,T2,T3,T4,T5,T6,T7,T8,T9>
  >
    : or_<
        is_unwrappable<T0>,
        is_unwrappable<T1>,
        is_unwrappable<T2>,
        is_unwrappable<T3>,
        is_unwrappable<T4>,
        is_unwrappable<T5>,
        is_unwrappable<T6>,
        is_unwrappable<T7>,
        is_unwrappable<T8>,
        is_unwrappable<T9>
      >
{};

would get simplified to (as is done on my branch):

// specialize is_unwrappable
// a tuple is_unwrappable if any of its elements is_unwrappable
template<typename... Types>
  struct is_unwrappable<
    thrust::tuple<Types...>
  >
    : or_<
        is_unwrappable<Types>...
      >
{};

In my opinion, if you already require C++11 anyway, I would suggest getting started on such changes now, wherever the public tuple interface is used, since they're independent of the particular tuple implementation.

[alliepiper]

In my opinion, if you already require C++11 anyway, I would suggest getting started on such changes now, wherever the public tuple interface is used, since they're independent of the particular tuple implementation.

Ah, I see what you mean.

Agreed! Modernizations that don't depend on the tuple implementation should be fine to work on now.