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

[FR] Add lower_upper constrain #1438

Open
andrjohns opened this issue Jul 15, 2024 · 5 comments
Open

[FR] Add lower_upper constrain #1438

andrjohns opened this issue Jul 15, 2024 · 5 comments
Labels
feature New feature or request good first issue Good for newcomers

Comments

@andrjohns
Copy link
Contributor

With the merge of stan-dev/math#3087, the lub_* constrain and free functions now support a single tuple of bounds. It would be great to add the lower_upper bounds type so that both bounds can be specified in a single function call.

Example pseudo-stan:

functions {
  (vector, vector) make_bounds(...) { ... }
}

...

parameters {
  vector<lower_upper = make_bounds(...)> x;
}

This would be great for situations where parameter bounds depend on functions of other parameters, and there is some non-trivial computation shared by both bounds

@andrjohns andrjohns added the feature New feature or request label Jul 15, 2024
@WardBrian
Copy link
Member

I believe we still need signatures in the serializer/deserializer in the Stan repo, no?

@andrjohns
Copy link
Contributor Author

I believe we still need signatures in the serializer/deserializer in the Stan repo, no?

Ah good point, will do now

@andrjohns
Copy link
Contributor Author

@WardBrian having some troubles adding a single tuple overload for read_free_lub, since it results in an ambiguous signature where the first element of the Sizes... is treated as the upper bound.

Would it be safe to add a separate read_free_lub_tuple signature instead (same for write and constrain)?

@WardBrian
Copy link
Member

Need to think about the implementation a bit. Might be worth starting in stanc3 and seeing which is more natural:

  1. The naming structure you suggest. The backend will need special logic to make sure this is called
  2. Just generating a temporary auto temp__ = lub; then calling the normal read_free_lub(sizes, get<0>(lub), get<1>(lub)) (this means the function just added to Stan-Math isn't necessary).

I think my preference would be number 1, but happy to discuss either.

@andrjohns
Copy link
Contributor Author

No worries! I don't really have a preference either way, so I'm happy with whichever option would be easiest on the implementation side

@WardBrian WardBrian added the good first issue Good for newcomers label Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants