tuple_io + tuple_cat

[andrewcorrigan]

Getting the ball rolling on this pull request. There are two pieces:

1. tuple_io, which is a minimal modification of code from [1] to enable stream support for thrust::tuple. Documentation of this functionality is provided in [2]
2. tuple_cat. I am including code from @bryancatanzaro [3], as per the request of @jaredhoberock in the thrust-users discussion [4].

[1] https://github.com/boostorg/tuple
[2] http://www.boost.org/doc/libs/1_55_0/libs/tuple/doc/tuple_users_guide.html#streaming
[3] https://github.com/bryancatanzaro/tuple-cat
[4] https://groups.google.com/d/msg/thrust-users/NxMTkwPp2bg/-s79wXylAREJ

[jaredhoberock]

Hi Andrew, thanks for owning this. I'll provide comments inline under the "files changed" tab.

[jaredhoberock]

We don't require a new public header to support this functionality; simply including thrust/tuple.h should suffice.

Let's move this file to thrust/detail/tuple/tuple_io.h, and add #include <thrust/detail/tuple/tuple_io.h> to the end of thrust/tuple.h.

[andrewcorrigan]

sounds good to me. Just want to double-check. tuple_io.h pulls in istream, ostream and locale. Is it an issue that thrust/tuple.h would be pulling those in as well?

[jaredhoberock]

Yes, that's OK

[jaredhoberock]

tuple_io.h looks mostly fine after some formatting changes. I'm most concerned about the use of thrust::detail::cons in the implementation. Someday we may wish to switch the definition of thrust::tuple to something liketemplate<class... T> tuple = using std::tuple, and this implementation of tuple_io.h would interfere with that. See how tuple_cat is specially implemented to avoid using anything beyond the stuff in the public interface of thrust::tuple.

The implementation of thrust::tuple has grown into quite a few files at this point, so let's introduce the directory thrust/detail/tuple, and move the following files into that directory:

• thrust/detail/tuple.inl (perhaps rename this to thrust/detail/tuple.h, as it's not really an .inl file)
• thrust/detail/tuple_meta_transform.h
• thrust/detail/tuple_transform.h
• thrust/detail/tuple_cat.h
• thrust/detail/tuple_io.h

Finally, before this pull request can be merged, the functionality needs documentation and tests. Function prototypes should be provided in thrust/tuple.h with Doxygen, and I'd add a few unit tests to testing/testing.cu. The test programs in bryancatanzaro/tuple_cat will be a good starting point for testing tuple_cat.

[andrewcorrigan]

Jared,

Thanks for the feedback and answering my follow-up questions. I'll address each of these issues asap.

[andrewcorrigan]

A work-in-progress. I still need to deal with cons, null_type, and reorganize file structure, and also the copyright statement for tuple_cat, but I dealt with namespaces, newlines, and moving tuple_manipulator out of the global thrust namespace.

[andrewcorrigan]

Latest fixes:

• Fixed more newlines
• Moved everything requested into tuple/detail/tuple_detail
• Both tuple_io.h and tuple_cat.h and included at the end of tuple.h
• Fixed syntax errors and warnings in tuple_helpers.h @bryancatanzaro

Anything else besides unit tests and documentation?

[jaredhoberock]

eliminate commented out code

[jaredhoberock]

Looks good. I'd eliminate the commented-out code which causes syntax errors. Unit tests & documentation should be all that is left to do.

[andrewcorrigan]

I finally had some spare time to work on this. I just added doxygen for tuple_io and tuple_cat. I put the prototypes after the includes, otherwise there are things from thrust::detail::tuple_detail that won't be defined yet. Any issues with what I did?

I'll get to unit testing as soon as I can, although I'm not sure when that will be..

[jaredhoberock]

Thanks Andrew, looks good.

[andrewcorrigan]

unit tests are added.

I had to compile the unit tests manually.. I tried with scons, using the latest upstream revision, but even after bypassing many errors, I couldn't get it to work. Is it supposed to work on mac 10.10 + cuda 7?

[andrewcorrigan]

I'm really sorry that I took forever to get this finished. Are you still interested in this PR? If there's anything I can do to help, please let me know.

[brycelelbach]

This was closed when we pushed the new Git master history to GitHub. Please rebase onto the new master and we'll take a look.

[brycelelbach]

So we're just gonna replace our tuple with a C++11 tuple or cuda::std::tuple, so I don't think this will make sense going forward.

[andrewcorrigan]

@brycelelbach Great to hear there may be some progress on that front. Will there be streaming/IO support using either of those tuple implementations (which I proposed to add via this PR)?

[brycelelbach]

Hey Andrew! Not at first, but we'll be open to PRs for that - if you want to rebase this onto top of trunk and reopen it we will be happy to merge it as is.