Unify ODE solution types: DAESolution, RODESolution as ODESolution aliases#1261
Open
ChrisRackauckas-Claude wants to merge 1 commit intoSciML:masterfrom
Open
Unify ODE solution types: DAESolution, RODESolution as ODESolution aliases#1261ChrisRackauckas-Claude wants to merge 1 commit intoSciML:masterfrom
ChrisRackauckas-Claude wants to merge 1 commit intoSciML:masterfrom
Conversation
…or ODESolution ODESolution now carries the union of all fields from the previous separate types: - `du` (from DAESolution) for derivative values - `W` (from RODESolution) for noise process - `seed` (from RODESolution) for random seed DAESolution and RODESolution are now const aliases for ODESolution. All build_solution dispatches create ODESolution directly. calculate_solution_errors! is unified with problem-type dispatch. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
|
RFC @oscardssmith @TorkelE @isaacsas should we do this with the major v7 coming up? It would improve the autodiff code a lot. |
Member
|
Is the improvement just that there would be less duplication? |
Member
|
yes |
Member
|
Seems reasonable to me, but I don't use DAESolution or RODESolution, so don't have a great idea of how disruptive this would be. |
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.
Summary
ODESolutionto carry the union of all fields from the previously separate types:du(derivatives, from DAESolution),W(noise process, from RODESolution), andseed(random seed, from RODESolution)DAESolutionandRODESolutionare nowconstaliases forODESolution, eliminating redundant struct definitionsbuild_solutiondispatches createODESolutiondirectly, with problem-type dispatch determining which fields are populatedcalculate_solution_errors!is unified into a single implementation that branches on problem type (AbstractDAEProblem,AbstractRODEProblem/AbstractSDDEProblem,AbstractDDEProblem, or standard ODE)dae_solutions.jlandrode_solutions.jl(duplicategetproperty,solution_new_retcode,solution_new_tslocation,solution_slice,ConstructionBasemethods, callable methods, etc.)isdenseplotnow checkssol.probtype instead ofsol isa AbstractRODESolutionBreaking changes
ODESolutionstruct has new fields (du,W,seed) which changes its type parametersDAESolutionorRODESolutionwill now match anyODESolution(since they are aliases)AbstractDAESolutionorAbstractRODESolutionwith concrete solution values will no longer match (since the concrete type is alwaysODESolution <: AbstractODESolution)AbstractDAESolution,AbstractDDESolution,AbstractRODESolutionare unchanged and remain in the hierarchy for interface purposesTest plan
🤖 Generated with Claude Code