refactor: Convert *Problem constructors to accept SciMLProblemOptions - #4901
Open
AayushSabharwal wants to merge 1 commit into
Open
refactor: Convert *Problem constructors to accept SciMLProblemOptions#4901AayushSabharwal wants to merge 1 commit into
AayushSabharwal wants to merge 1 commit into
Conversation
Extends the SciMLFunctionOptions options-struct pattern (already applied to *Function constructors) down through DiscreteProblem, ImplicitDiscreteProblem, IntervalNonlinearProblem, DAEProblem, ODEProblem, SteadyStateProblem, SDEProblem, DDEProblem, SDDEProblem, NonlinearProblem, NonlinearLeastSquaresProblem, HomotopyProblem, OptimizationProblem, BVProblem, and LinearProblem: each now has a public `(sys, op, tspan, opts::SciMLProblemOptions)` entry point alongside its kwargs-based wrapper. `__process_SciMLProblem` gains an `options_struct = Val(true)` mode that calls the target `*Function`'s own opts-accepting method directly instead of its permissive kwargs wrapper, fixing a latent bug where a `*Problem` constructor naming a `SciMLFunctionOptions` field explicitly (e.g. `sparse`) would consume it out of the kwargs pass-through before it reached the inner `*Function` (concretely: `LinearProblem(...; sparse = true)` never reached `LinearFunction`). Since the opts-accepting methods have no such catch-all, bespoke keywords that used to ride through harmlessly (`steady_state`, `seed`, `noise`, `constant_lags`, `checkvars`, `_skip_tstops`, ...) are now explicitly intercepted at each call site and either discarded or re-merged into the final constructor call as needed. Also: BVProblem now stores `check_compatibility`/`t` in the options struct itself (via a new shared `maybe_derive_t_from_tspan` helper) rather than as bespoke keywords; removes the unused `cse` keyword from HomotopyProblem; fixes `constant_lags` on DDEProblem/SDDEProblem to resolve symbolic parameter references against the built `p` instead of silently dropping them; and removes the now-unnecessary `kwargs...` from `maybe_build_initialization_problem`, closing off a path where bespoke keywords could leak into the initialization subproblem's own Function construction. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| args = (; f, u0, p) | ||
|
|
||
| maybe_codegen_scimlproblem(expression, SteadyStateProblem{_iip}, args; kwargs...) | ||
| maybe_codegen_scimlproblem(Val{E}, SteadyStateProblem{_iip}, args; kwargs...) |
Contributor
There was a problem hiding this comment.
[Runic] reported by reviewdog 🐶
Suggested change
| maybe_codegen_scimlproblem(Val{E}, SteadyStateProblem{_iip}, args; kwargs...) | |
| return maybe_codegen_scimlproblem(Val{E}, SteadyStateProblem{_iip}, args; kwargs...) |
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
1 participant
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.
Extends the SciMLFunctionOptions options-struct pattern (already applied to *Function constructors) down through DiscreteProblem, ImplicitDiscreteProblem, IntervalNonlinearProblem, DAEProblem, ODEProblem, SteadyStateProblem, SDEProblem, DDEProblem, SDDEProblem, NonlinearProblem, NonlinearLeastSquaresProblem, HomotopyProblem, OptimizationProblem, BVProblem, and LinearProblem: each now has a public
(sys, op, tspan, opts::SciMLProblemOptions)entry point alongside its kwargs-based wrapper.__process_SciMLProblemgains anoptions_struct = Val(true)mode that calls the target*Function's own opts-accepting method directly instead of its permissive kwargs wrapper, fixing a latent bug where a*Problemconstructor naming aSciMLFunctionOptionsfield explicitly (e.g.sparse) would consume it out of the kwargs pass-through before it reached the inner*Function(concretely:LinearProblem(...; sparse = true)never reachedLinearFunction). Since the opts-accepting methods have no such catch-all, bespoke keywords that used to ride through harmlessly (steady_state,seed,noise,constant_lags,checkvars,_skip_tstops, ...) are now explicitly intercepted at each call site and either discarded or re-merged into the final constructor call as needed.Also: BVProblem now stores
check_compatibility/tin the options struct itself (via a new sharedmaybe_derive_t_from_tspanhelper) rather than as bespoke keywords; removes the unusedcsekeyword from HomotopyProblem; fixesconstant_lagson DDEProblem/SDDEProblem to resolve symbolic parameter references against the builtpinstead of silently dropping them; and removes the now-unnecessarykwargs...frommaybe_build_initialization_problem, closing off a path where bespoke keywords could leak into the initialization subproblem's own Function construction.Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Add any other context about the problem here.