fix(families): raise ValueError for x outside support in exponential and uniform#118
fix(families): raise ValueError for x outside support in exponential and uniform#118MyznikovFD wants to merge 1 commit into
Conversation
MyznikovFD
requested review from
Desiment and
LeonidElkin
and removed request for
LeonidElkin
MyznikovFD
added
Bug
LeonidElkin
left a comment
LeonidElkin
left a comment
There was a problem hiding this comment.
The issue explicitly lists normal.py, but this commit does not add any support validation for the normal score. Even though all finite real values are in support, ContinuousSupport() excludes ±inf and NaN, and the current implementation returns inf/nan gradients for those inputs. Either add a finite-value check here or clarify why normal is intentionally out of scope.
| grad_a = np.full_like(x, 1.0 / width) | ||
| grad_b = np.full_like(x, -1.0 / width) |
There was a problem hiding this comment.
np.full_like preserves the dtype of x. For integer inputs, 1.0 / width and -1.0 / width are cast to integers, so the score becomes [[0, 0], ...] instead of the expected floating-point gradient. Force a floating dtype here, e.g. np.full_like(x, ..., dtype=np.float64), and add a regression test with integer x.
| if np.any(x < 0): | ||
| raise ValueError(f"Score is undefined for x < 0 (outside support). Got x = {x}") |
There was a problem hiding this comment.
This check only rejects x < 0, but the family support excludes non-finite values as well. For example, ContinuousSupport(left=0.0).contains(np.inf) is false, while score([np.inf]) currently returns -inf instead of raising. Reject non-finite x values, ideally with a test for np.inf and np.nan.
| if np.any((x < a) | (x > b)): | ||
| raise ValueError(f"Score is undefined for x outside support [{a}, {b}]. Got x = {x}") |
There was a problem hiding this comment.
This support check does not reject NaN: both (x < a) and (x > b) are false for NaN, so score([np.nan]) returns a regular-looking gradient. Since NaN is not contained in the distribution support, reject non-finite values before computing the gradient.
| ValueError | ||
| If any value in `x` lies outside the distribution's support. |
There was a problem hiding this comment.
This should be documented under a NumPy-style Raises section. Right now ValueError is placed directly after Returns, so the docstring structure is invalid and may render incorrectly in generated docs.
2 participants
No description provided.