stan-dev / stanc3

The Stan transpiler (from Stan to C++ and beyond).
BSD 3-Clause "New" or "Revised" License
140 stars 44 forks source link

Expose std_normal_log_qf #1213

Closed spinkney closed 1 year ago

spinkney commented 2 years ago

A couple of questions for @WardBrian when I go to expose this.

This is a new suffix type with _qf. In fact, this is a new, new type with _log_qf. I believe I'd add the below to keep things consistent. I can change or add if you or others disagree.

WardBrian commented 2 years ago

The code snippet you posted would make it so that _log_qf and _qf always must both exist when one does. I am not 100% sure, but I don’t think we would want that? I think if we’re following the example of things like poisson_log_lpmf (which I think is a valid comparison but let me know if I’m wrong), then the “suffix” is still just _qf and the distribution name is itself “std_normal_log”.

The other question here is I think whether we would want this as part of the declarative structure or if we would just want to ad-hoc add these functions? This is a tricky call. If we expect there to be more than a few of them, AND if they have easily predictable signatures (e.g. knowing the signature for x_lpdf, can I write down the signature for x_qf) then it should be declarative. It might end up like RNG where some are declarative and some are added separately - I am personally fine with this, but if we can do them all declaratively I think that would be better.

spinkney commented 2 years ago

The other difference is that these are vectorized. Do I put it with the distributions or with the functions?

WardBrian commented 2 years ago

That should be fine, again assuming the arguments/return types are "nice" in some sense.

I think my preference is these end up with the distributions in the declarative list. If you can't make that work but want them in for the release, I'd also be fine with just exposing the signatures this needs manually for now and then figuring out a better encapsulation later, probably when we want to add the second _qf. Fair?