Document direct operator-splitting developer interfaces - #108
Conversation
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
| 1. define `SciMLBase.isadaptive(::MyAlgorithm) = true`, | ||
| 2. define [`OrdinaryDiffEqOperatorSplitting.alg_adaptive_order`](@ref), the order of | ||
| its error estimator, and | ||
| 3. write the tolerance-scaled error estimate to `parent.EEst` at the end of |
There was a problem hiding this comment.
No. This should be done with the new interface (set_EEst or so) which we recently added to OrdinaryDiffEqCore.
There was a problem hiding this comment.
Done in 050a1fb — the guidance, the code block, and the _perform_step! docstring now go through OrdinaryDiffEqCore.set_EEst!, with a note that it and get_EEst are the interface and the field location is an implementation detail.
Worth flagging that the package was already inconsistent with itself here: src/integrator.jl defines OrdinaryDiffEqCore.set_EEst!/get_EEst for the split integrator types and the controllers read the estimate through them, but the built-in PalindromicPairLieTrotterGodunov still assigned the field directly. Since the docs cite it as the complete example, I changed it too, plus a stale comment in integrator.jl describing the estimate as "written to the node's EEst".
I left the two direct writes in reinit_node_controller! alone — those are the implementation side of the hook writing the package's own field, so routing them through set_EEst! would just call back into the same assignment. Say the word if you'd rather they were uniform.
Verified locally on Julia 1.12.6: full Core group passes (adaptivity 70/70 covers the PPLTG estimate path), strict docs build exit 0, Runic clean.
`dotview` and `broadcastable` are public in `Base.Broadcast` but not in `Base`, so accessing them through `Base` made strict QA flag them. Qualify all four broadcast overloads through `Base.Broadcast`. `materialize!` and the `Broadcasted` type it dispatches on have no public spelling in either module and no alias, so they stay as documented QA exceptions -- a bare `run_qa` cannot pass while `TreeOption` implements `.=`. `failfactor_default` joins them until a registered OrdinaryDiffEqCore carries SciML/OrdinaryDiffEq.jl#4111. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
The adaptive-algorithm guidance told extension authors to assign `parent.EEst` directly, even though the package already defines `OrdinaryDiffEqCore.set_EEst!`/`get_EEst` for its integrator types and the controllers read the estimate through them. Route the documented interface, its docstring, and the built-in PalindromicPairLieTrotterGodunov that the docs cite as a complete example through `set_EEst!` instead, so the field location stays an implementation detail. The direct writes in `reinit_node_controller!` are left as they are: those are the implementation side of the hook, on the package's own field. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Summary
Strict QA
The bare
run_qa(OrdinaryDiffEqOperatorSplitting)failed on five names, not one. They fall into two groups.Fixed by accessing the owner.
dotviewandbroadcastableare public inBase.Broadcastbut not inBase, andsrc/config_tree.jlreached them throughBase. All four broadcast overloads now qualify throughBase.Broadcast, which drops both findings with no exception entry.Genuinely non-public, so allowed. On Julia 1.12.6:
BaseBase.BroadcastdotviewbroadcastableBroadcastStylematerialize!Broadcastedmaterialize!andBroadcastedhave no public spelling in either module and no alias, so an empty exception list is unreachable whileTreeOptionimplements.=. They are allowed with the reason inline intest/qa/qa.jl.failfactor_defaultjoins them until a registered OrdinaryDiffEqCore carries SciML/OrdinaryDiffEq.jl#4111 — that one is temporary.Note that SciMLTesting #43/#44 do not affect any of this; they change the Aqua subprocess load path and the
persistent_tasksallowance.Verification
GROUP=Core Pkg.test()on Julia 1.12.6 — all groups pass (adaptivity70,alias_u013,backward35,callbacks77,config_tree140,consistency27,convergence90,failure_escalation20,operator_splitting_api1342,saving122,sync9)test/qa/qa.jlon Julia 1.12.6 (21/21) and 1.10.11 LTS (19/19)src,test,docsDependencies
failfactor_defaultentryIgnore until reviewed by @ChrisRackauckas.