Replace existing TensorSpec bounds before packing new values - #5
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 #4.
Change
Clear each existing bound after validation and before packing its replacement. The numeric repeated-field packers append values, so repeated
set_boundscalls previously retained stale values and could make scalar or vector bounds unreadable. The int8/uint8 byte packers already replaced their contents.The setter now replaces bounds consistently across numeric types.
Nonestill unsets the corresponding bound, invalid updates leave the message unchanged, and the low-level packers' behavior remains intact. No signature or protocol changes are included.Verification
All 32 new regression/control cases pass using real protobuf messages and the public bounds reader. They cover scalar/vector transitions, idempotence, eight numeric types, independently clearing bounds, variable-length specs, exact serialized messages and unchanged state after invalid updates. Original code fails 19 cases and passes thirteen controls.
The complete repository suite passes 417 tests and fourteen 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.
Both jobs test
519950a576e9ce0e02092f79ed1d941ead9090a2, reproduce the original failures, pass the original existing suite, restore the submitted bytes and rerun the regressions. Direct wheel builds pass, and all 32 cases pass against installed wheels outside the checkout. Formatting, lint, dependency and patch checks pass. Logs, JUnit, coverage and packages are workflow artifacts; validation configuration remains on a separate fork branch.Scope
An existing packaging issue prevents building a wheel from the source distribution because it omits the required api-common-protos submodule. This reproduces with original production source in both hosted environments. Direct wheel builds from the initialized checkout pass. Existing NumPy warnings remain.
Only the bounds setter and new regression module change. No production dependencies, upstream workflows or existing test expectations are modified. Tests include local client/server examples, without live external environments.
Run
python -m pytest dm_env_rpc/v1/tensor_spec_bounds_replacement_test.pyfor the regressions.