Skip to content

Conversation

@yizhang-yiz
Copy link

@yizhang-yiz yizhang-yiz commented Jan 11, 2022

This PR addresses #1091.

new integration tests:

  • test/integration/good/dae_good.stan
  • test/integration/bad/variadic_dae/*.stan

Submission Checklist

  • Run unit tests
  • Documentation
    • If a user-facing facing change was made, the documentation PR is here: DAE doc docs#467
    • OR, no user-facing changes were made

Release notes

Support Differential-Algebraic Equation solver.

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)

@WardBrian WardBrian self-requested a review January 11, 2022 21:31
@WardBrian
Copy link
Member

Do you have any sense of a timeline on stan-dev/math#2644?

#1027 is nearing completion, and whichever one of these merges first will create problems for the other (the typechecking and code gen of variadic functions is more or less completely rewritten in #1027)

@yizhang-yiz
Copy link
Author

Do you have any sense of a timeline on stan-dev/math#2644?

From the initial feedback from @charlesm93 it should be soon. Of course the final decision is his.

Copy link
Member

@WardBrian WardBrian left a comment

Choose a reason for hiding this comment

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

One minor comment, otherwise LGTM

This was linked to issues Jan 12, 2022
@WardBrian WardBrian mentioned this pull request Jan 12, 2022
6 tasks
Copy link
Member

@WardBrian WardBrian left a comment

Choose a reason for hiding this comment

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

I think these comments in Stan_math_signatures provide more clutter than illumination.

Other than that, LGTM. I will approve/merge when the stan::math PR is merged and docs are approved

@WardBrian
Copy link
Member

Looks like you'll need to update to the new pattern introduced in #1004 - should be pretty easy to see based on the variadic ode, again

Copy link
Member

@WardBrian WardBrian left a comment

Choose a reason for hiding this comment

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

Should be good to go after the math branch is merged

@rok-cesnovar
Copy link
Member

Tests pass so feel free to merge.

@WardBrian
Copy link
Member

Doc is under review still, but I think it will definitely get in before the release so we can merge

@WardBrian WardBrian merged commit 6c2bc5e into master Jan 19, 2022
@WardBrian WardBrian deleted the add_dae_support branch March 17, 2022 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support DAE signatures expose DAE solver interface to Stan

4 participants