Adding Dirichlet moment and tests #5174
Conversation
Codecov Report
@@ Coverage Diff @@
## main #5174 +/- ##
==========================================
- Coverage 78.09% 78.06% -0.04%
==========================================
Files 88 88
Lines 14149 14157 +8
==========================================
+ Hits 11050 11052 +2
- Misses 3099 3105 +6
|
|
Looks great Larry! Thanks a lot |
|
Would be great to have your help for some of the other multivariate distributions |
| norm_constant = at.sum(a, axis=-1)[..., None] | ||
| moment = a / norm_constant | ||
| if not rv_size_is_none(size): | ||
| if isinstance(size, int): |
There was a problem hiding this comment.
Actually we may need to revisit this. When size is a number it is still inside a TensorConstant. Not sure what isinstance(size, int) will do. Maybe we should check for size.ndim?
Size may also be a symbolic shape. So the same reasoning applies.
There was a problem hiding this comment.
Looks great Larry! Thanks a lot
As always, thanks to you for all your help.
Should we change isinstance(size, int) to if isinstance(size, int) or size.ndim > 0? Would that be a correct approach?
There was a problem hiding this comment.
I checked again and I think that checks will never evaluate true. Even when size is an integer, it gets converted to a TensorConstant([size]):
size = x.owner.inputs[1]
isinstance(size, int) # False
size.data # array([2])If I remove the isinstance block, all tests still pass
This pull request adds
get_momentto theDirichletclass and corresponding tests.Regarding tests, my third test fails as I am unsure if I am understanding broadcasting correctly. Hoping to add at least a fourth test upon passing the third one
CC: @ricardoV94