Fix FieldError for Magnus/Linear integrators with OrdinaryDiffEqDiffe…#3464
Open
singhharsh1708 wants to merge 2 commits intoSciML:masterfrom
Open
Fix FieldError for Magnus/Linear integrators with OrdinaryDiffEqDiffe…#3464singhharsh1708 wants to merge 2 commits intoSciML:masterfrom
singhharsh1708 wants to merge 2 commits intoSciML:masterfrom
Conversation
…rentiation (SciML#3232) Magnus integrators (MagnusGL6, MagnusGL8, etc.) and other OrdinaryDiffEqLinearExponentialAlgorithm subtypes have no `autodiff` field — only `krylov`, `m`, `iop`. When OrdinaryDiffEqDifferentiation is loaded (e.g. via DifferentialEquations.jl), the generic `_alg_autodiff(::OrdinaryDiffEqExponentialAlgorithm{CS,AD})` dispatch would call `alg.autodiff` on these types, causing a FieldError crash. Fix: - Import OrdinaryDiffEqLinearExponentialAlgorithm into OrdinaryDiffEqDifferentiation - Add _alg_autodiff(::OrdinaryDiffEqLinearExponentialAlgorithm) returning Val{false}(), intercepting calls before the generic ExponentialAlgorithm dispatch that accesses the nonexistent field - The existing prepare_alg override in OrdinaryDiffEqCore (line 298) already handles the prepare_alg path correctly Also adds regression tests verifying _alg_autodiff, prepare_alg, and forwarddiffs_model all work correctly for LinearExponentialAlgorithm subtypes.
ba144dc to
ff465b8
Compare
7272537 to
8f47539
Compare
Contributor
Author
|
Replaced the mock struct with real integrators (MagnusGL6, MagnusGL4, MagnusGL8, LieEuler, CG2, CG3, CG4a) and moved the regression test to OrdinaryDiffEqLinear/test/linear_method_tests.jl — no Project.toml changes needed. All 7 pass locally. |
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.
Fix
FieldErrorfor Magnus/Linear integrators whenOrdinaryDiffEqDifferentiationis loadedFixes #3232
Problem
Solving a non-autonomous linear ODE with any Magnus integrator (e.g.
MagnusGL6,MagnusGL8,LieEuler,CG2, etc.) crashes with aFieldErrorwhenOrdinaryDiffEqDifferentiationis loaded (which happens automatically viaDifferentialEquations.jl):Root Cause
MagnusGL6 <: OrdinaryDiffEqLinearExponentialAlgorithm <: OrdinaryDiffEqExponentialAlgorithm{0, false, ...}In
OrdinaryDiffEqDifferentiation/src/alg_utils.jl, the generic dispatch:catches all
OrdinaryDiffEqLinearExponentialAlgorithmsubtypes because they inherit fromOrdinaryDiffEqExponentialAlgorithm{0, false, ...}. These algorithms are matrix-exponential methods that never use ForwardDiff/FiniteDiff — they only have fieldskrylov,m, andiop.Fix
Add a more-specific
_alg_autodiffoverride that interceptsOrdinaryDiffEqLinearExponentialAlgorithmbefore the genericExponentialAlgorithmdispatch:This is consistent with how
OrdinaryDiffEqCorealready handles these types inprepare_algandhas_autodiff.Changes
OrdinaryDiffEqDifferentiation.jlOrdinaryDiffEqLinearExponentialAlgorithmfrom Corealg_utils.jl_alg_autodiffoverride returningVal{false}()differentiation_traits_tests.jl_alg_autodiff,prepare_alg, andforwarddiffs_modelTesting
All 3 regression assertions pass with clean precompilation: