Skip to content

fix transform inits input and output sizes#993

Merged
rok-cesnovar merged 1 commit into
stan-dev:masterfrom
SteveBronder:fix/transform-inits-return-size
Oct 7, 2021
Merged

fix transform inits input and output sizes#993
rok-cesnovar merged 1 commit into
stan-dev:masterfrom
SteveBronder:fix/transform-inits-return-size

Conversation

@SteveBronder
Copy link
Copy Markdown
Contributor

Fix for transform inits so that the input vector is the same as the number of constrained parameters while the output vector is the size of the number of unconstrained params

Submission Checklist

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

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)

@WardBrian WardBrian linked an issue Oct 6, 2021 that may be closed by this pull request
@WardBrian
Copy link
Copy Markdown
Member

OCaml looks fine to me, can't comment much on the generated C++/whether this resolves the issue. I assume you tried it out?

@SteveBronder
Copy link
Copy Markdown
Contributor Author

Yeah this fixes the issue in #992, tested out that model. As the summary says that goof happened because of the difference in the number of constrained and unconstrained parameters for things like simplex and unit vectors

@wds15
Copy link
Copy Markdown

wds15 commented Oct 7, 2021

As this PR is scheduled to make up a 2.28.1... I would ask to hold for a moment that release so that we also fix the issues seen in the Adjoint ODE solver if possible... at least I would like to try a debug session today and if it turns out that the problem can be fixed in finite time, then that would ideally also be part of 2.28.1?

@rok-cesnovar
Copy link
Copy Markdown
Member

Yeah, definitely, we can wait. Steve will bring up the id=1 thing (event though I think we have come to an agreement) on today's meeting so we were planning on holding off at least until after the meeting. Will follow the Discourse thread for updates on the adjoint ODE issue. If you think the issue is too big for quick fix, we will release 2.28.1 with this PR.

@wds15
Copy link
Copy Markdown

wds15 commented Oct 7, 2021

Yup... I don't want to delay too much, but if there is a chance to include a fix, then that's great. If not we go for 2.28.2 or we see.

@SteveBronder
Copy link
Copy Markdown
Contributor Author

@rok-cesnovar if you approve I think this is good!

@rok-cesnovar rok-cesnovar merged commit 763421b into stan-dev:master Oct 7, 2021
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.

Issue with simplex_free in 2.28.0

4 participants