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

Add Lambert W x F distributions to XGBoostLSS #65

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

gmgeorg
Copy link

@gmgeorg gmgeorg commented Nov 30, 2023

Adds skewed and tail Lambert W x F distributions as an option to XGBoostLSS.

In particular, adds Lambert W x {Normal, Weibull, Gamma, Exponential, LogNormal} distributions. Adds examples to notebooks and adds its own California Housing example notebook.

image

@StatMixedML
Copy link
Owner

@gmgeorg Thanks for creating the PR.

I assume you further refine the notebooks and the .py files? Since, e.g., the Gaussian example still has some explanations for modelling the gaussian distribution whereas it should explain the Lambert W x F distributions. Also, result wise, the gaussian example notebook does not look good.

@gmgeorg
Copy link
Author

gmgeorg commented Dec 6, 2023

@StatMixedML yes will clean up. However, I wanted to send this out first as a general "it works" proof of concept PR. If this looks good to you high level with the current changes, I will continue and
a) clean up .py files
b) update notebooks accordingly
c) fix / add unit tests based on the changes made (in particular dist_select() and changing the 1:1 map between filenames and distribution names to allow for more than 1 distribution class per file)

Let me know if I should revisit/re-implement differently any proposed changes; if not, will go ahead and clean up rest of PR for a e2e review.

@gmgeorg
Copy link
Author

gmgeorg commented Dec 25, 2023

@StatMixedML updated PR with changes addressing earlier points. PTAL

@fkiraly
Copy link

fkiraly commented Jan 26, 2024

@gmgeorg, would you be interested adding some distributions to skpro?

See proposed integration issue here: sktime/skpro#184

Integration with skpro would pull all the way through to sktime.

@gmgeorg
Copy link
Author

gmgeorg commented Jan 28, 2024

@fkiraly Great idea! Will take a look and add an issue there.

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.

None yet

3 participants