Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: encapsulate variadic functions in typechecking #1259

Merged
merged 3 commits into from
Oct 14, 2022

Conversation

WardBrian
Copy link
Member

In anticipation of #576 and potential future variadic higher-order functions, I decided it was worth taking a good look at cleaning up the shared code between existing variadic functions.

The DAE and ODE functions, and the future solve_newton etc are completely described by a few rules:

  1. What type does the higher-order function itself return?
  2. What arguments are for this function, rather than passed to the first argument (I named these the control_args)?
  3. What return type do we require for the function passed in?
  4. What, if any, arguments do we require for the function passed in?

This PR reorganizes the existing code around this to remove current duplication and prevent more going foward.

Note that the above are not sufficient to capture reduce_sum, since item 4 is actually dynamic depending on the first argument. So, for now at least, this is left as a special case.

This would also let us try to print these as part of --dump-stan-math-signatures eventually, if we wanted to.

Finally, I'd like to note that it would eventually be nice to also clean up the code gen for these higher-order functions, which is currently also very special cased due to the differing positions of arguments, different requirements for functors being generated, etc.

Submission Checklist

  • Run unit tests
  • Documentation
    • OR, no user-facing changes were made

Release notes

Replace this text with a short note on what will change if this pull request is merged. This will be included in the release notes.

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

reduce_sum is not just variadic, but also polymorphic, so it is left as a special case
@WardBrian WardBrian added the cleanup Code simplification or clean-up label Oct 13, 2022
src/frontend/Typechecker.ml Outdated Show resolved Hide resolved
src/frontend/Typechecker.ml Outdated Show resolved Hide resolved
src/middle/Stan_math_signatures.ml Outdated Show resolved Hide resolved
@WardBrian WardBrian merged commit ab791f7 into master Oct 14, 2022
@WardBrian WardBrian deleted the refactor/encapsulate-variadics branch October 14, 2022 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code simplification or clean-up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants