Add CombineAs<R>() parameters generator

[Stoorx]

CombineTo() allows the user to combine two or more sequences to produce values of a Cartesian product of those sequences' elements converted to the required type. The behavior of this function is the same as already implemented and well-known Combine().

CombineTo() allows to use struct-parametrized tests directly instead of dealing with tuples. Maybe the better naming for this function is CombineAs()

The behavior of other functions have not been changed (according to the tests passing). The additional basic test cases are provided for this feature. Feel free to give me some ideas to make better coverage.

Dear maintainers, please note, this feature is very necessary in GROMACS project and we want it as soon as possible. I see that 1.16.0 has been released. Maybe it is possible to make the new release faster and include this feature in the upcoming release somehow? Or maybe backport it as 1.16.1 too?

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[Stoorx]

@derekmauro As soon as I understand you are the one of maintainers. Can you please review this feature and help to bring it faster to the release? We really need it in GROMACS.

[Stoorx]

Rebased onto main and adapted to use the new version of ConvertGenerator

[Stoorx]

@zhanyongwan @derekmauro Hello! Please take a look at this feature. We really need it in GROMACS.

[zhanyong-wan]

Hi @Stoorx , I'm not an owner of this project. Let me talk to Derek about this. Meanwhile, I suppose you can have a local workaround by defining this in your own code? Thanks.

[Stoorx]

Hello! Thank you so much!

local workaround by defining this in your own code?

Yes, we can use the ConvertGenerator<T>, but it requires to define the constructor which unpacks a tuple element by element. It is a kind of annoying boilerplate IMO. Please take a look:

struct KernelInputParameters
{
    using TupleT = std::tuple<Nbnxm::KernelType, CoulombKernelType, int>;
    //! The kernel type and cluster pair layout
    Nbnxm::KernelType kernelType;
    //! The Coulomb kernel type
    CoulombKernelType coulombKernelType;
    //! The VdW interaction type
    int vdwKernelType;
    KernelInputParameters(TupleT t) :
        kernelType(std::get<0>(t)), coulombKernelType(std::get<1>(t)), vdwKernelType(std::get<2>(t))
    {
    }
};

This pull request proposes the CombineTo<R>() (naming is a discussion thing) function, which does these conversions automagically (actually some braced-initialization tricks). So the struct is not required to provide a tuple constructor anymore.

Thank you!

[derekmauro]

I believe this is from GCC14 building in C++20 mode:

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
googletest/test/googletest-param-test-test.cc: In member function 'virtual void CombineToTest_CopyConstructible_Test::TestBody()':
googletest/test/googletest-param-test-test.cc:649:38: error: no matching function for call to 'CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(<brace-enclosed initializer list>)'
  649 |       Values(CopyConstructible{0, "A"}, CopyConstructible{1, "B"}));
      |                                      ^
googletest/test/googletest-param-test-test.cc:637:5: note: candidate: 'constexpr CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(const CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible&)'
  637 |     CopyConstructible(const CopyConstructible& other) = default;
      |     ^~~~~~~~~~~~~~~~~
googletest/test/googletest-param-test-test.cc:637:5: note:   candidate expects 1 argument, 2 provided
googletest/test/googletest-param-test-test.cc:649:65: error: no matching function for call to 'CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(<brace-enclosed initializer list>)'
  649 |       Values(CopyConstructible{0, "A"}, CopyConstructible{1, "B"}));
      |                                                                 ^
googletest/test/googletest-param-test-test.cc:637:5: note: candidate: 'constexpr CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(const CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible&)'
  637 |     CopyConstructible(const CopyConstructible& other) = default;
      |     ^~~~~~~~~~~~~~~~~
googletest/test/googletest-param-test-test.cc:637:5: note:   candidate expects 1 argument, 2 provided
googletest/test/googletest-param-test-test.cc:650:66: error: no matching function for call to 'CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(<brace-enclosed initializer list>)'
  650 |   CopyConstructible expected_values[] = {CopyConstructible{0, "A"},
      |                                                                  ^
googletest/test/googletest-param-test-test.cc:637:5: note: candidate: 'constexpr CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(const CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible&)'
  637 |     CopyConstructible(const CopyConstructible& other) = default;
      |     ^~~~~~~~~~~~~~~~~
googletest/test/googletest-param-test-test.cc:637:5: note:   candidate expects 1 argument, 2 provided
googletest/test/googletest-param-test-test.cc:651:66: error: no matching function for call to 'CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(<brace-enclosed initializer list>)'
  651 |                                          CopyConstructible{1, "B"}};
      |                                                                  ^
googletest/test/googletest-param-test-test.cc:637:5: note: candidate: 'constexpr CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(const CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible&)'
  637 |     CopyConstructible(const CopyConstructible& other) = default;

[zhanyong-wan]

Thanks!

[Stoorx]

I believe this is from GCC14 building in C++20 mode:

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
googletest/test/googletest-param-test-test.cc: In member function 'virtual void CombineToTest_CopyConstructible_Test::TestBody()':
googletest/test/googletest-param-test-test.cc:649:38: error: no matching function for call to 'CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(<brace-enclosed initializer list>)'
  649 |       Values(CopyConstructible{0, "A"}, CopyConstructible{1, "B"}));
      |                                      ^
googletest/test/googletest-param-test-test.cc:637:5: note: candidate: 'constexpr CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(const CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible&)'
  637 |     CopyConstructible(const CopyConstructible& other) = default;
      |     ^~~~~~~~~~~~~~~~~
googletest/test/googletest-param-test-test.cc:637:5: note:   candidate expects 1 argument, 2 provided
googletest/test/googletest-param-test-test.cc:649:65: error: no matching function for call to 'CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(<brace-enclosed initializer list>)'
  649 |       Values(CopyConstructible{0, "A"}, CopyConstructible{1, "B"}));
      |                                                                 ^
googletest/test/googletest-param-test-test.cc:637:5: note: candidate: 'constexpr CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(const CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible&)'
  637 |     CopyConstructible(const CopyConstructible& other) = default;
      |     ^~~~~~~~~~~~~~~~~
googletest/test/googletest-param-test-test.cc:637:5: note:   candidate expects 1 argument, 2 provided
googletest/test/googletest-param-test-test.cc:650:66: error: no matching function for call to 'CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(<brace-enclosed initializer list>)'
  650 |   CopyConstructible expected_values[] = {CopyConstructible{0, "A"},
      |                                                                  ^
googletest/test/googletest-param-test-test.cc:637:5: note: candidate: 'constexpr CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(const CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible&)'
  637 |     CopyConstructible(const CopyConstructible& other) = default;
      |     ^~~~~~~~~~~~~~~~~
googletest/test/googletest-param-test-test.cc:637:5: note:   candidate expects 1 argument, 2 provided
googletest/test/googletest-param-test-test.cc:651:66: error: no matching function for call to 'CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(<brace-enclosed initializer list>)'
  651 |                                          CopyConstructible{1, "B"}};
      |                                                                  ^
googletest/test/googletest-param-test-test.cc:637:5: note: candidate: 'constexpr CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible::CopyConstructible(const CombineToTest_CopyConstructible_Test::TestBody()::CopyConstructible&)'
  637 |     CopyConstructible(const CopyConstructible& other) = default;

I have no idea how to fix it. It seems to me that the code is valid and should not fail. Can you give me some advise?

[Stoorx]

I can not see the CI build logs since it refers to some internal domain 'fusion2'. Maybe you could provide some instructions to reproduce it locally or give me the full log?

[zhanyong-wan]

This syntax is invalid as CopyConstructible has a user-defined ctor. It must be constructed using a constructor as CopyConstructible(...) instead of CopyConstructible{...}.

Please define a CopyConstructible(int, std::string) ctor to be used this.

[Stoorx]

Done. Please check

[Stoorx]

Hello! Are there any updates for this PR? Can you please take a look at it? @zhanyong-wan

[zhanyong-wan]

Hi Stoorx, I'm not an owner of the repo and don't have the power to approve PRs. We'll need @derekmauro to do the final review. Thanks.