Add support for scatter() on lists-of-struct columns

[mythrocks]

Addendum to #6768, to support scatter on columns of type list<struct>.

Depends on #6768.

The hard part was the tests.

(This branch is based off of that in #6768, and shows all commits from that PR. Only the last 4 commits are material to struct_view support.)

[GPUtester]

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

[mythrocks]

The for-loop at scatter.cuh#L662 couldn't be boiled down to a 2-sequence std::transform() without obfuscating the logic with a bunch of captures. I'm open to suggestions there. :/
Edit: How fortunate that @codereport is tagged on this too. :]

[mythrocks]

I'll leave this as a draft PR until its dependency (#6768) is committed.

[codecov]

Codecov Report

Merging #6817 (cc73681) into branch-0.17 (f3b0e06) will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           branch-0.17    #6817   +/-   ##
============================================
  Coverage        81.94%   81.94%           
============================================
  Files               96       96           
  Lines            16164    16164           
============================================
  Hits             13246    13246           
  Misses            2918     2918           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3b0e06...cc73681. Read the comment docs.

[mythrocks]

I'll leave this as a draft PR until its dependency (#6768) is committed.

#6768 has been committed. I'll wait for the CI to complete on this PR, and switch out of DRAFT mode.

[harrism]

Looks good. A few small comments.

[codereport]

Overall looks good. For the unit tests, I find them pretty hard to read the way they are written, with all of the varying "parameter list indentation" and vertical lists for initializer lists, i.e.:

 auto expected_strings =
    strings_column_wrapper{{"eight",
                            "eight",
                            "eight",
                            "one",
                            "one",
                            "nine",
                            "nine",
                            "nine",
                            "nine",
                            "three",
                            "three",
                            "four",
                            "four",
                            "five",
                            "five"},
                           make_counting_transform_iterator(0, [](auto i) { return i != 1; })};

It's hard to give comments though on changes to make. Ideally though, I aim for unit tests that end up looking like this:

TYPED_TEST(FixedPointTypes, FixedPointSimple)
{
  using namespace numeric;
  using decimalXX  = TypeParam;
  using RepType    = cudf::device_storage_type_t<decimalXX>;
  using fp_wrapper = cudf::test::fixed_point_column_wrapper<RepType>;

  auto const mask     = cudf::test::fixed_width_column_wrapper<bool>{0, 1, 1, 1, 0, 0};
  auto const a        = fp_wrapper{{110, 220, 330, 440, 550, 660}, scale_type{-2}};
  auto const b        = fp_wrapper{{0, 0, 0, 0, 0, 0}, scale_type{-2}};
  auto const expected = fp_wrapper{{0, 220, 330, 440, 0, 0}, scale_type{-2}};
  auto const result   = cudf::copy_if_else(a, b, mask);

  CUDF_TEST_EXPECT_COLUMNS_EQUAL(expected, result->view());
}

I might set up a follow up PR / give a better code talk at some point to share my thoughts / see what people think.

[mythrocks]

@codereport, I agree about the indentation in the tests. These did not start out looking this way.I can try disabling the code formatter around the column definitions.

[codereport]

@codereport, I agree about the indentation in the tests. These did not start out looking this way.I can try disabling the code formatter around the column definitions.

Yea, my recommendation would be to try and write the code in a way that plays well with clang-format. I intentionally keep my "literal" tests (aka tests that have input literally written out) small so that they don't line break. For "comprehensive" unit tests, i do the same with thrust iterators.

[mythrocks]

Yea, my recommendation would be to try and write the code in a way that plays well with clang-format.

I'm not sure to what degree this can be done, given the complexity of list<struct> construction. I'm in the process of giving this a go. I should have an updated version shortly.

P.S. I wasn't a fan of how clang-format lands up removing the list grouping in the child columns. I'm putting this back in manually.

[mythrocks]

I should have an updated version shortly.

Done. Resolved merge conflicts, and added (arguably) better formatting for literal columns used in list<struct> tests.

[harrism]

I don't like turning clang-format off for more than a few lines. If someone comes in and changes some code and relies on clang-format to clean it up, we will end up with an accretion of terrible formatting. If it's too verbose to create a list<struct>, then use aliases and/or write a convenience wrapper.

[mythrocks]

I don't like turning clang-format off for more than a few lines...

I've now restricted where clang-format is turned off to just the literal definitions, so that it's still clear where the list rows start/end.

[harrism]

I don't like turning clang-format off for more than a few lines...

I've now restricted where clang-format is turned off to just the literal definitions, so that it's still clear where the list rows start/end.

Thanks, that is much nicer.