Add functors for the embedded Laplace approximation#1513
Conversation
Typechecking for laplace_marginal is in principle done but needs a lot of care for better error messages
|
As of stan-dev/math@80904889a2, this is now passing CI |
SteveBronder
left a comment
SteveBronder
left a comment
There was a problem hiding this comment.
Few Qs but things look good!
| match tes with | ||
| | {expr= Variable lik_fun; _} :: lik_tupl :: tes -> | ||
| let lik_fun, lik_tupl = | ||
| needs_higher_order_autodiff lik_fun; |
There was a problem hiding this comment.
[minor] It would be nice to have a comment here that this changes an object at the global scope
src/frontend/Typechecker.ml
Outdated
| let cov_fun_type, cov_tupl = | ||
| check_function_callable_with_tuple cf tenv id cov_fun cov_tupl | ||
| (UnsizedType.ReturnType UMatrix) in | ||
| (* note for future: pred_rng typechecking would need to look at second tuple here *) |
There was a problem hiding this comment.
| (* note for future: pred_rng typechecking would need to look at second tuple here *) | |
| (* note for future: pred_rng typechecking would need to look at tuple for the training prediction and test prediction *) |
| (* Function is non existent *) | ||
| Semantic_error.invalid_tilde_no_such_dist loc name | ||
| (List.hd_exn argumenttypes |> snd |> UnsizedType.is_int_type) | ||
| |> error | ||
| (* The laplace_marginal helpers are not in the | ||
| environment lookup, so we patch them back here *) | ||
| if Stan_math_signatures.is_embedded_laplace_fn (id.name ^ "_lupmf") then | ||
| let id = {id with name= id.name ^ "_lupmf"} in | ||
| let fn_expr = | ||
| check_laplace_fn ~is_cond_dist:true loc cf tenv id arguments in | ||
| match fn_expr.expr with | ||
| | CondDistApp (kind, _, args) -> (args, kind) | ||
| | _ -> | ||
| Common.ICE.internal_compiler_error | ||
| [%message | ||
| "Typechecking a laplace marginal did not return a distribution " | ||
| (fn_expr : Ast.typed_expression)] | ||
| else | ||
| (* Otherwise, the function is non existent *) |
There was a problem hiding this comment.
Can we not add these because they take in tuples of variadic size so they do not fit in the distributions list? It would be nice if we could even make a separate list for them instead of catching a failure
There was a problem hiding this comment.
Correct -- because they allow any function for the covariance, they also take in any length of tuple the user wants
SteveBronder
left a comment
SteveBronder
left a comment
There was a problem hiding this comment.
Few small comments but I think this is good to merge as is! 🥳
3 participants
This is a draft until stan-dev/math#3097 is updated and merged. It could probably also do with some more code-gen tests.
Similar to reduce_sum, these functions have too many rules to fit into the general variadic framework, so they have custom typechecking and errors, which make up most of this PR.
Closes #1511
Submission Checklist
Release notes
Added new functions for the embedded Laplace approximation.
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)