Skip to content

Adding skew_double_exponential family#1845

Draft
trinhdhk wants to merge 5 commits intopaul-buerkner:masterfrom
trinhdhk:NEW-skew-double-exponential
Draft

Adding skew_double_exponential family#1845
trinhdhk wants to merge 5 commits intopaul-buerkner:masterfrom
trinhdhk:NEW-skew-double-exponential

Conversation

@trinhdhk
Copy link
Copy Markdown

@trinhdhk trinhdhk commented Dec 19, 2025

Adding experimental skew_double_exponential family, targeting #1836
Speed gains about 25%.
Caveat: currently I have noticed that sigma in Stan's implementation is twice larger than its counterpart in asym_laplace. This is a breaking change in terms of interpretation (and prior specs), so I'm still keeping asym_laplace.
I added one test but I think we need to add further test to ensure equivalence.
Please check if I added the test in the correct directory (currently in local test). And kindly let me know.
And sorry for CLRF (if there's still left-over).

@paul-buerkner
Copy link
Copy Markdown
Owner

I am fine with replacing asym_laplace Stan code with the native implementation in Stan. I would prefer not to add a new family for this. That causes too much confusion.

Do you know why one has sigma while the other has sigma / 2? Which one is the more "natural" parameterization?

@trinhdhk
Copy link
Copy Markdown
Author

trinhdhk commented Jan 2, 2026 via email

@paul-buerkner
Copy link
Copy Markdown
Owner

Let's keep the current brms asym_laplace parameterization then but use Stan's built-in functions for fitting. Would you mind editing your PR accordingly? I am sorry that this means that we won't need most of the code you currently included in the PR.

@trinhdhk
Copy link
Copy Markdown
Author

trinhdhk commented Jan 2, 2026

Let's keep the current brms asym_laplace parameterization then but use Stan's built-in functions for fitting. Would you mind editing your PR accordingly? I am sorry that this means that we won't need most of the code you currently included in the PR.

I'm happy to but it does mean a breaking change for past studies that have used asym laplace. For a mature software, this is not a good practice.

Most of my code now is just a wrapper for asym_laplace so an inplace replacement would reduce the time for testing equivalence etc. But like I said, I was thinking of the researchers (me).

@paul-buerkner
Copy link
Copy Markdown
Owner

What would it break exactly?

@trinhdhk
Copy link
Copy Markdown
Author

trinhdhk commented Jan 2, 2026

What would it break exactly?

Ah I misread. Did you mean doubling sigma before feeding into Stan so that we keep the current formulation?

@paul-buerkner
Copy link
Copy Markdown
Owner

Correct.

@trinhdhk
Copy link
Copy Markdown
Author

trinhdhk commented Jan 2, 2026 via email

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.

2 participants