Validate DiscreteArray maxima against integer dtype limits - #23
Open
sylvesterkaczmarek wants to merge 1 commit into
Open
sylvesterkaczmarek wants to merge 1 commit into
sylvesterkaczmarek wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #22.
Change
Compare
num_values - 1withnp.iinfo(dtype).maxinstead of comparing NumPy dtypes. The previous check could accept a maximum outside a signed integer's range. With NumPy 1.26 this can construct an invalid spec, such asDiscreteArray(129, np.int8)with maximum -128; with NumPy 2.5 it reaches a cast and raisesOverflowErrorinstead of the documented diagnostic.This is a one-line validation correction. Valid signed/unsigned ranges, including their exact upper endpoints, remain accepted. Public signatures, generated values, replacement and serialization behavior are otherwise unchanged.
Verification
All 38 new regression/control cases pass. They cover all eight integer dtypes, exact endpoints, out-of-range counts, dtype strings/objects, NumPy integer counts, replacement and pickle round trips. Original production code fails 14 cases and passes 24 controls.
The complete repository suite passes 202 tests and six subtests locally on macOS/Python 3.12 and in exact-commit hosted validation on Ubuntu with Python 3.10/NumPy 1.26.4 and Python 3.12/NumPy 2.5.3. The hosted jobs test
3335013894f56fb598ef49ad60753c5ddde9fedf, reproduce the original failures, pass the original existing suite, restore the submitted bytes and rerun the regressions.Source-distribution and wheel builds pass. All 38 cases also pass against installed wheels outside the checkout with the import location verified. New-test formatting, lint, dependency and patch checks pass. Logs, JUnit reports, coverage and packages are workflow artifacts; the validation workflow stays on a separate fork-only branch.
Run the regression module with
python -m pytest dm_env/discrete_array_bounds_test.py.Only the specification implementation and new test module change. No production dependencies, upstream workflows or existing test expectations are modified.