[ENH] Weibull distribution#242
Conversation
fkiraly
left a comment
There was a problem hiding this comment.
Thanks! Looks great!
- I'd name the parameters differently. The "lambda" or rate parameter is typically called the scale parameter, and identical with your current
scaleparameter. Your currentlambda_valis the power or shape parameter, I would not call this "lambda" because that's typically the other parameter (your currentscale). - we should probably deal with negative
x. Everthing is either 0 or na there. - some further remarks on explicit forms below
| def log_pdf(self, x): | ||
| """Logarithmic probability density function.""" | ||
| d = self.loc[x.index, x.columns] | ||
| lpdf_arr = np.log(d.lambda_val / d.scale) + (d.lambda_val - 1) * np.log(x.values / d.scale) - (x.values / d.scale) ** d.lambda_val |
There was a problem hiding this comment.
I think you can cancel some of the logs here
There was a problem hiding this comment.
Is there any log that we can cancel? I can only think of storing division calculation
There was a problem hiding this comment.
Ahh I think I got it
There was a problem hiding this comment.
I am not really sure which one is simpler
or
I think it is the first option as it offers a simpler equation(?)
There was a problem hiding this comment.
I think the first option is numerically more stable, becaues over the course of the calculation your numbers stay in a narrower range.
Ahh yes, thank you so much for pointing this out!! I think previously I read the formula description reversed. I have adjusted this. Once again, thank you so much for pointing this out! |
|
Great! Let us know when you would like a review. |
Reference Issues/PRs
#22
What does this implement/fix? Explain your changes.
Weibull probability distribution
Does your contribution introduce a new dependency? If yes, which one?
no
What should a reviewer concentrate their feedback on?
Did you add any tests for the change?
pip install --editable .[dev,test]and create a simple driver program to use this new distributionAny other comments?
PR checklist
For all contributions
How to: add yourself to the all-contributors file in the
skproroot directory (not theCONTRIBUTORS.md). Common badges:code- fixing a bug, or adding code logic.doc- writing or improving documentation or docstrings.bug- reporting or diagnosing a bug (get this pluscodeif you also fixed the bug in the PR).maintenance- CI, test framework, release.See here for full badge reference
For new estimators
docs/source/api_reference/taskname.rst, follow the pattern.Examplessection.python_dependenciestag and ensureddependency isolation, see the estimator dependencies guide.