initial gamma distribution #8
Conversation
|
Just wondering if these kind of PR are welcome here? If so I can also implement other distributions e.g. #21, else we can close this? |
| # pylint: disable=too-many-function-args | ||
| super().setUp(gamma.Gamma) | ||
| self.assertion_fn = lambda x, y: np.testing.assert_allclose(x, y, rtol=RTOL) | ||
|
|
There was a problem hiding this comment.
Could you also add tests for the properties concentration and rate? Just to check that the broadcasting is working as intended.
There was a problem hiding this comment.
@franrruiz I am not sure what you mean here precisely?
There was a problem hiding this comment.
I just meant to add a test where concentration has shape (say) [4, 2] and rate is [2]. We would test that distribution.concentration coincides with the input value, and distribution.rate is [4, 2] and has been broadcasted properly. (And the same for the reverse, where the broadcasting happens on concentration). This might seem like a simple thing to test but it's helpful mainly for maintenance.
|
thanks @franrruiz i'll fix it up and ping you |
|
@franrruiz I would be ready for another review... I think I have addressed all your issues... thank you so much! |
|
thanks @hbq1 |
|
Apologies for the delay with merging your PR, and thanks a lot for sending it in @kashif! |
No description provided.