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

Expose additional normal_id_glm pointwise signatures #1354

Open
andrjohns opened this issue Aug 31, 2023 · 4 comments
Open

Expose additional normal_id_glm pointwise signatures #1354

andrjohns opened this issue Aug 31, 2023 · 4 comments
Labels
feature New feature or request good first issue Good for newcomers

Comments

@andrjohns
Copy link
Contributor

The current normal_id_glm signatures for use with a univariate outcome y only support matrix type inputs for x, for example:

real normal_id_glm_lpdf(real y | matrix x, real alpha, vector beta, real sigma)

For pointwise usage, it would be great to add corresponding row_vector signatures:

real normal_id_glm_lpdf(real y | row_vector x, real alpha, vector beta, real sigma)

As the current signatures require calling to_matrix() at each iteration:

generated quantities {
  vector[N] log_lik;

  for (n in 1:N) {
    log_lik[n] = normal_id_glm_lpdf(y[n] | to_matrix(x[n]), alpha, beta, sigma);
  }
}
@andrjohns andrjohns added the feature New feature or request label Aug 31, 2023
@WardBrian
Copy link
Member

Is this just for convenience or do we expect different performance? I thought to_matrix was essentially free for eigen types

@andrjohns
Copy link
Contributor Author

Mostly convenience, since the majority of use-cases with a univariate outcome would be a single set of predictors (e.g., pointwise log-likelihood), so a real y and row_vector x combo would be more common than a real y and matrix x

@WardBrian WardBrian added the good first issue Good for newcomers label Sep 26, 2024
@Pranavchiku
Copy link

I would be happy to help in this, if any reference PR or code exists please share.

@WardBrian
Copy link
Member

Hi @Pranavchiku --

It appears that the C++ math library already supports this, so the change in the compiler would be relatively tiny. The existing code in the compiler is here:

; ([Lpdf], "normal_id_glm", [DVector; DMatrix; DReal; DVector; DReal], SoA)

We want to change DMatrix there to another option. Here are the current ones:

type dimensionality =
| DInt
| DReal
| DVector
| DMatrix
| DIntArray
(* Vectorizable int *)
| DVInt
(* Vectorizable real *)
| DVReal
| DVComplex
(* DEPRECATED; vectorizable ints or reals *)
| DIntAndReals
(* Vectorizable vectors - for multivariate functions *)
| DVectors
| DDeepVectorized
| DComplexVectors
| DDeepComplexVectorized
[@@warning "-37"]

It looks like none of these are exactly what we want, so we'd create a new enum variant. I'd suggest the name DEigenTypes. And then we need to update expand_arg:

let rec expand_arg = function

such that DEigenTypes gets expanded to [UMatrix; UVector; URowVector]

Finally, to test that this all compiles, we'd then add calls to all the new overloads (including existing permutations) to this file:
https://github.com/stan-dev/stanc3/blob/master/test/integration/good/function-signatures/distributions/univariate/continuous/normal/normal_id_glm.stan

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

3 participants