Skip to content

Add tuple overload and cleanup code for promote_scalar #2706

Merged
rok-cesnovar merged 7 commits into
developfrom
feature/promote_scalar_tuple
Jul 7, 2022
Merged

Add tuple overload and cleanup code for promote_scalar #2706
rok-cesnovar merged 7 commits into
developfrom
feature/promote_scalar_tuple

Conversation

@SteveBronder
Copy link
Copy Markdown
Collaborator

Summary

This adds an overload to promote_scalar for the compiler that allows the input to be a set of tuples and the promotion type to be a tuple of scalar types. So for the compiler impl of tuples we will be able to generate code such as

std::tuple<std::vector<var>, var> = 
  promote_scalar<std::tuple<var, var>(std::vector<double>{...}, double(2.0));

This also cleanups the code for promote_scalar a little bit. Instead of having to do some roundabouts using a static struct method for doing actually the promotions we called from promote_scalar we just have explicit free functions for the particular cases we need that can be called from one another.

Tests

Tests are added for testing promotions of tuples with various inner types and scalar types. A test is also added for the new type trait is_tuple<> which just detects if a template is a tuple or not.

./runTests.py ./test/unit/math/ -f promote_scalar_test
./runTests.py ./test/unit/math/ -f is_tuple

Side Effects

Nope

Release notes

Add tuple overload and cleanup code for promote_scalar

Checklist

  • Math issue #

  • Copyright holder: Steve Bronder

    The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
    - Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
    - Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

  • the basic tests are passing

    • unit tests pass (to run, use: ./runTests.py test/unit)
    • header checks pass, (make test-headers)
    • dependencies checks pass, (make test-math-dependencies)
    • docs build, (make doxygen)
    • code passes the built in C++ standards checks (make cpplint)
  • the code is written in idiomatic C++ and changes are documented in the doxygen

  • the new changes are tested

@WardBrian WardBrian linked an issue Apr 15, 2022 that may be closed by this pull request
@WardBrian
Copy link
Copy Markdown
Member

WardBrian commented Apr 15, 2022

Just to check, does this do the right thing if it is called with a std::vector of tuples?

E.g, given a
std::vector<std::tuple<int,int>> vector;
Does the call
promote_scalar<std::tuple<double, double>>(vector)

Work like it does when the vector contains non-tuples (element wise?)

@SteveBronder
Copy link
Copy Markdown
Collaborator Author

No it did not, good catch! But I just put in an update that fixes that with a test for it as well

@WardBrian
Copy link
Copy Markdown
Member

I mentioned this in the assign PR, but it is worth testing/making sure we support at least the following:

a tuple
An array of tuples
A tuple which contains an array
A tuple which contains a tuple
A tuple which contains an array of tuples

The kind of recursive logic needed to get all that right usually generalizes to even deeper nesting

@SteveBronder
Copy link
Copy Markdown
Collaborator Author

I'll double check but looking at the test I believe I cover all of those

@WardBrian WardBrian requested a review from rok-cesnovar April 27, 2022 13:19
@rok-cesnovar
Copy link
Copy Markdown
Member

@SteveBronder is this ready for review? I have been tagged for this a few weeks ago - just want to make sure.

Copy link
Copy Markdown
Member

@rok-cesnovar rok-cesnovar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. I went ahead and merge recent develop in as the branch was pretty old at this point. Merge away assuming the tests now pass.

@rok-cesnovar rok-cesnovar merged commit 57fe69b into develop Jul 7, 2022
@rok-cesnovar rok-cesnovar deleted the feature/promote_scalar_tuple branch July 7, 2022 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

Support std::tuple types in promote_scalar

4 participants