Repository navigation
Process bounds via get_internal_bounds on the converter fast path - #730
Merged
timmens merged 3 commits intoNov 4, 2026
Merged
Conversation
When params is a 1d numpy array and no constraints or scaling are used, get_converter took a fast path that called .astype(float) directly on bounds.lower and bounds.upper. Bounds given as a list or tuple therefore raised AttributeError, while the same bounds worked on the slow path. Delegate to get_internal_bounds, which already has its own fast path for 1d array bounds and handles other pytrees, so both paths behave the same. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
Member
|
Thanks for the excellent PR! The fix looks good, and the tests make the intended behavior clear. While reviewing, I noticed that the existing test uses |
Contributor
Author
|
Renamed both tests to fast_path and merged main, thanks! |
timmens
approved these changes
Nov 4, 2026
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.
Closes #725
What problem do you want to solve?
When params is a 1d numpy array and there are no constraints and no scaling, get_converter takes a fast path that calls astype(float) directly on bounds.lower and bounds.upper. So bounds given as a list or tuple (including the deprecated lower_bounds=[...]) raise AttributeError. The same bounds work fine once scaling is turned on, because that goes through the slow path.
The fix makes the fast path call get_internal_bounds, the same function the slow path uses. It already has a quick branch for array bounds, so array input behaves the same as before. As a side effect, the fast path now also gets the lower > upper check.
I added a parametrized test in test_conversion.py for list, tuple and np.ndarray bounds. The list and tuple cases fail on main and pass with this change. test_conversion.py and test_bounds.py pass locally (41 passed), and ruff is clean.
Thanks @timmens for the quick reply. #729 only touches bounds.py, so it doesn't conflict with this.
AI use
I used Claude Code to help find the bug, write the reproducer, and draft the fix and test. I reviewed every line of the diff, and I ran the reproducer and the tests myself before and after the change.
Todo
Disclosure: found, reproduced and the draft fix tested locally with the help of an AI coding agent (Claude Code); reviewed before filing.