fix(core): report output_dim, not min_output_dim, when it is too small - #44
Open
ChrisW09 wants to merge 1 commit into
Open
fix(core): report output_dim, not min_output_dim, when it is too small#44ChrisW09 wants to merge 1 commit into
ChrisW09 wants to merge 1 commit into
Conversation
ChrisW09
force-pushed
the
fix/floor-error-names-output-dim
branch
2 times, most recently
from
July 27, 2026 20:48
714dd2b to
40b265f
Compare
``_resolve_output_bounds`` reported every floor violation against
``min_output_dim``, but ``lo`` is set to ``output_dim`` on the non-adaptive
branch. So setting ``output_dim=0`` blamed a parameter the caller had not
touched -- and one that is ignored entirely when ``adaptive`` is False:
Preprocessor(numerical_method="ple", output_dim=0)
-> min_output_dim must be >= 1, got 0.
Fix: raise min_output_dim to at least the family minimum.
Following that advice would not have helped. Families that pre-validate
``output_dim`` themselves (cubicspline, bspline) already reported it correctly,
so the two halves of the package disagreed.
Name the parameter that produced the value.
ple output_dim=0 -> output_dim must be >= 1, got 0.
cubicspline output_dim=0 -> output_dim must be >= 3 for the cubic spline basis, got 0
adaptive, min_output_dim=0 -> min_output_dim must be >= 1, got 0.
Closes #38
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ChrisW09
force-pushed
the
fix/floor-error-names-output-dim
branch
from
July 27, 2026 20:49
40b265f to
825a053
Compare
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 #38.
Problem
AdaptiveResolutionMixin._resolve_output_boundsreported every floor violation againstmin_output_dim, butlois assignedoutput_dimon the non-adaptive branch a few linesearlier. So a user who set
output_dimgot told about a parameter they never touched — andone that is ignored entirely when
adaptive=False:Acting on that "Fix:" line would not have helped.
The two halves of the package also disagreed: families that pre-validate
output_dimthemselves reported it correctly, while those relying on the shared resolver did not.
Fix
Pick the name from the branch that produced the value —
min_output_dimonly whenadaptiveis set and the caller supplied one, otherwiseoutput_dim. The "Fix:" linefollows suit.
Tests
Three added to
tests/test_exceptions.py: the non-adaptive message namesoutput_dim, itdoes not mention
min_output_dim, and the adaptive path still namesmin_output_dimwhenthat is genuinely what was set.
Cosmetic, but it is the first thing a user sees when they mis-size a transformer, and it was
pointing at the wrong knob.
Full suite: 483 passed, 9 xfailed.
ruff checkclean on changed files; pyright unchanged atits pre-existing 71 errors.
🤖 Generated with Claude Code