Families/exponential family configuration #63
Conversation
LeonidElkin
left a comment
LeonidElkin
left a comment
There was a problem hiding this comment.
Divide the changes into refactoring and features, you shouldn't lump everything together.
| with np.errstate(divide="ignore", invalid="ignore"): | ||
| return np.where(p < 1.0, -np.log(1.0 - p) / lambda_, np.inf) |
There was a problem hiding this comment.
-log(1-p)/lambda == -log1p(-p)/lambda, but log1p is more accurate near p=1
| result = np.where( | ||
| np.abs(t_arr) < 1e-12, | ||
| 1.0 + 0j, | ||
| lambda_ / denominator, | ||
| ) | ||
| return cast(ComplexArray, result) |
There was a problem hiding this comment.
1e-12 is a magic number. Make a variable inside the configure_exponential_family() function as you did it in tests
| def _exponential_support(_: Parametrization) -> ContinuousSupport: | ||
| """Support of exponential distribution""" | ||
| return ContinuousSupport(left=0.0, right=math.inf) |
There was a problem hiding this comment.
right=math.inf is redundant here. We already have right=inf by default in ContinuousSupport .
| def test_analytical_computations_caching(self): | ||
| """Test that analytical computations are properly cached.""" | ||
| comp = self.exponential_family(lambda_=1.0).analytical_computations | ||
|
|
||
| expected_chars = { | ||
| CharacteristicName.PDF, | ||
| CharacteristicName.CDF, | ||
| CharacteristicName.PPF, | ||
| CharacteristicName.CF, | ||
| CharacteristicName.MEAN, | ||
| CharacteristicName.VAR, | ||
| CharacteristicName.SKEW, | ||
| CharacteristicName.KURT, | ||
| } | ||
| assert set(comp.keys()) == expected_chars |
There was a problem hiding this comment.
Here and in similar tests in other families, there is a relic of the past in the form of a naming. Caching is no longer checked here, we need to rename it to something that reflects the point.
| def test_array_input_support(self): | ||
| """Test that PDF supports array inputs.""" | ||
| dist = self.exponential_family(lambda_=1.0) | ||
| x_array = np.array([-1.0, 0.0, 0.5, 1.0, 2.0, 3.0]) | ||
|
|
||
| pdf = dist.query_method(CharacteristicName.PDF) | ||
| pdf_array = pdf(x_array) | ||
|
|
||
| assert pdf_array.shape == x_array.shape | ||
| scipy_pdf = expon.pdf(x_array, scale=1.0) # scale = 1/lambda = 1.0 | ||
|
|
||
| self.assert_arrays_almost_equal(pdf_array, scipy_pdf) |
There was a problem hiding this comment.
Here and in similar tests in other families, why are we checking only PDF?
…families are divided into separate files
MyznikovFD
force-pushed
the
families/exponential-family-configuration
branch
from
4d203cb to
7de7ffe
Compare
Implemented an exponential parametric family
File with implementations of concrete parametric families is separated into 3 files: normal.py, uniform.py, exponential.py
New files are located in their own directory builtins/contitnuous