Provide a clear error when the solver is not instantiated.#762
Open
SwissChardLeaf wants to merge 1 commit into
Open
Provide a clear error when the solver is not instantiated.#762SwissChardLeaf wants to merge 1 commit into
SwissChardLeaf wants to merge 1 commit into
Conversation
Fixes patrick-kidger#705. Incorporates discussion from patrick-kidger#706.
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 #705. Incorporates discussion from #706.
Hello Diffrax community. This is my first contribution attempt!
Summary
When
solveris passed as a class rather than an instance (e.g.diffrax.EulerHeuninstead ofdiffrax.EulerHeun()),diffeqsolvepreviously failed with obscure errors such asAssertionErrororTypeError: functools.partial() argument after ** must be a mapping, not property.This adds an early
_validate_solvercheck that raises a clearValueErrorwith a concrete example.Relation to #706
This implements the approach discussed in #706, incorporating Patrick's review feedback on that PR:
issubclasswithisinstance(solver, type)so non-class inputs do not raiseTypeError.diffrax.Euler()in the error message rather thandfx.Euler().Design notes (open to feedback)
1. Widened
solverannotationThe
solverparameter is annotated asAbstractSolver | type[AbstractSolver]so that beartype allows uninstantiated solver classes through to_validate_solver, which then raises the intendedValueError. Without this, beartype rejects the call before our check runs.2.
validated_solverAfter validation, the solver is stored in a separate
validated_solver: AbstractSolvervariable used for the remainder ofdiffeqsolve. This works well with pyright, even with the widened input type annotation.