Change lmoments3 behaviour with fitkwargs #2045
Conversation
|
@aulemahal Sascha suggested an error instead of a warning, thoughts? Sometimes I'm not sure which is more appropriate |
There was a problem hiding this comment.
I was thinking of a warning because I was thinking of a usage where one calls the function multiple time with different distributions but the same other arguments. I realize now that this does not make much sense, woups.
Thinking of it, an error might indeed be better. Sorry for the confusion!
Co-authored-by: Pascal Bourgault <[email protected]>
|
Now, we can use PWM in SPI/SPEI functions directly. I added tests: The results with lmoments3 (PWM) / gamma is close to what we get with ML/gamma or APP/gamma, so I'm satisfied, things work well. |
|
Should be good to go, let's see the tests |
Pull Request Checklist:
number) and pull request (:pull:number) has been addedWhat kind of change does this PR introduce?
fitkwargsis given withlmomentsmethodgammadistribution is used withfloc: This is accepted and used to shift the distributionDoes this PR introduce a breaking change?
No
Other information:
I realize that SPI/SPEI is still heavily constrained in what distribution can be used. For now, only gamma/fisk (log-logistic) are allowed. At some point we could revisit this. I think we reached a point where we could be more permissive, since we ironed out many of the potential problems in these functions. There is still potential that problems could arise if we allow more distributions, but we might have to accept that some problems might arise when working with these optimization procedures, and fix them on the fly (or not, if it's just a limitation of the method).