Differentiate with respect to refraction model parameters - #119
Merged
Conversation
A dual valued pressure or temperature previously needed the observer lifted to the same dual type, and even then the keyword constructors rejected it. Three layers now promote instead: - HUGHES, BENNETT, SG2 and SPARefraction take mixed argument types. The keyword form that @kwdef generates routes through SPARefraction's three argument constructor, so it promotes too. - SolPos and ApparentSolPos promote over their fields, so apparent angles that carry a dual can join geometric angles that do not. - The vector paths size their StructVector from the observer's element type promoted against the refraction model's, since the observer's precision alone gives a container the results cannot convert into. The same type constructors stay more specific, so existing calls dispatch exactly as before. Tests cover the four parametric models against finite differences for both pressure and temperature, the promoting constructors, and the vector path's element type. Interpolated gains autodiff coverage as well: it is absent from test_algorithms() and so had none, though it already worked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KhdBYRmHazfXZHWCRWt5fT
The section on making the error controller see the forcing asserted that declaring discontinuities and adding a quadrature state both help, without showing what either buys. It now separates the two. The almanac sunrise and the model's own kink are seven minutes apart in Amsterdam at the solstice, because transit_sunrise_sunset returns the upper limb at -0.8333 degrees while max(0, sind(elevation)) breaks at geometric zero. The guide bisects inside the almanac bracket for the time the model actually breaks at, then draws every solver step as a rug under the forcing across four variants, with a zoom on sunrise where the mismatch is visible. A measured table against a reltol = 1e-13 reference gives the ranking. Kink placement is worth a factor of 125 on the temperature at slightly less cost than declaring nothing, since the rejections it avoids more than pay for the two steps it forces. The quadrature state is worth only a factor of two here and earns its keep only when the integrand depends on a state, which the guide now says, along with the trapezoid alternative for when it does not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KhdBYRmHazfXZHWCRWt5fT
MD060 requires the compact table style, which wants a space on each side of every pipe. The new table's delimiter row was written without them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KhdBYRmHazfXZHWCRWt5fT
Contributor
Benchmark Results (Julia v1)Time benchmarks
Memory benchmarks
|
Contributor
Benchmark Results (Julia vlts)Time benchmarks
Memory benchmarks
|
Contributor
Benchmark Results (Julia vpre)Time benchmarks
Memory benchmarks
|
test/.CondaPkg is a CondaPkg.jl generated environment, not shareable state. Nothing declares it, since there is no test/CondaPkg.toml, and nothing under test/ uses CondaPkg, PythonCall or PyCall. The tracked meta file is a binary cache holding absolute paths from whichever machine last resolved it. docs/.CondaPkg was already ignored, so the pattern is generalised rather than extended one directory at a time. The files stay on disk and CondaPkg regenerates them on demand. benchmark/.CondaPkg is tracked for the same reason and has the same problem, but benchmark/python.jl does use CondaPkg, so it is left alone here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KhdBYRmHazfXZHWCRWt5fT
The docstring and both guides stated that the result element type follows the Observer element type. That was only true because a mismatched refraction model used to be a MethodError, and it is now the promotion of the two, since the apparent angles are computed at the model's precision. Promotion is kept rather than narrowing the model's parameters to the observer's type, because narrowing would have to special case ForwardDiff.Dual to avoid destroying derivatives. Type stability tests grow to cover the case. The vector path sizes its StructVector from a type computed at call time, so it has to infer to a concrete element type or the whole vector API goes unstable, and that path had no coverage: the existing testset only exercised scalar calls with NoRefraction and DefaultRefraction, neither of which can widen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KhdBYRmHazfXZHWCRWt5fT
benchmark/python.jl does use CondaPkg, but the tracked environment cannot be what serves it: pixi.toml and pixi.lock contain no reference to solposx or pandas, the two packages python.jl installs. What is tracked is the Copier template's own environment of copier, openssl and libstdcxx, the same stale content that test/.CondaPkg held, and its meta file is a binary cache of absolute paths from another machine. CondaPkg resolves the real dependencies on demand, and there is no tracked benchmark/CondaPkg.toml declaring anything. The **/.CondaPkg/ pattern added in the previous commit already covers this directory, and the files stay on disk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KhdBYRmHazfXZHWCRWt5fT
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #119 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 20
Lines 751 766 +15
=========================================
+ Hits 751 766 +15 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Differentiating a solar position with respect to a refraction model's pressure or
temperature previously required lifting the
Observerto the same dual type, and eventhen the keyword constructors rejected it:
Three layers now promote instead:
HUGHES,BENNETT,SG2andSPARefractionaccept mixed argument types. The keywordform that
@kwdefgenerates routes throughSPARefraction's three argumentconstructor, so
SPARefraction(pressure = dual)promotes too.SolPosandApparentSolPospromote over their fields. This was the second failure,MethodError: no method matching ApparentSolPos(::Float64, ::Float64, ::Float64, ::Dual, ::Dual),since a
Float64observer gives three geometric angles that carry no dual and twoapparent ones that do.
StructVectorfrom the observer's element type promotedagainst the refraction model's. Without this the scalar path would work while
solar_position(obs, dts, alg, model)still failed, because the preallocated containercame from the observer's precision alone and the duals cannot convert into it.
The same type constructors remain strictly more specific, so every existing call
dispatches exactly as before and
HUGHES(101325.0, 10.0)never reaches the new method.The internal
refraction_eltypereturnsUnion{}for a model without numeric fields,which is neutral under
promote_type, soNoRefraction,DefaultRefraction,ARCHERand
MICHALSKYleave the observer's element type untouched.Tests
test/positioning/test-autodiff.jlgoes from 6 testsets to 12:keyword form,
SolPosandApparentSolPosfield promotion,d/dpressureandd/dtemperatureagainst finite differences for each model, and the vector path'spromoted element type with zero partials on the geometric angles and nonzero on the
apparent ones.
Interpolated— latitude, longitude and altitude against finite differences undertwo refraction models, gradient and Hessian against the exact
SPAit wraps, vector andin-place dual propagation with per-time partials checked against the scalar derivative,
and both
out_of_rangemodes.Interpolatedis absent fromtest_algorithms()and sohad no autodiff coverage, though it already worked: its interpolants cover only the
geocentric quantities, which depend on time alone, so the duals travel through the
topocentric half it shares with
SPA.The
solar_positiondocstring and the autodiff guide both documented the old contract oflatitude, longitude and altitude only, and are updated.
Full
Pkg.test()passes, including Aqua's ambiguity detection over the new overlappingconstructor methods, and
docs/make.jlbuilds clean.Behaviour change
The result element type used to be documented as following the
Observer{T}element type.That held only because a mismatched refraction model was a
MethodError, and it is now thepromotion of the two, since the apparent angles are computed at the model's precision:
Matched precision (
HUGHES{Float32}()) and parameterless models (ARCHER,MICHALSKY)keep
Float32, andDefaultRefractionbuilds its model at the observer's precision, sothe default path is untouched. Narrowing the model's parameters to the observer's type
instead would preserve the old rule, but it would have to special case
ForwardDiff.Dualto avoid destroying derivatives, which is the sort of AD specific carve-out this package
otherwise does without.
The
solar_positiondocstring, the numeric precision guide and the index all stated theold rule and are corrected, with a worked example in the guide.
Type stability
Verified unchanged: 48 of 48 algorithm and refraction combinations infer on both the scalar
and the vector path, allocations are byte identical to
main(scalar 0, a 24 elementvector 768 or 1280 B), the
Float32/Float64/BigFloatsweep gives identical resulttypes, and JET reports nothing. The vector path matters most here, since
_result_eltypecomputes a type at call time and an unfolded result would have made the whole vector API
unstable.
test-typestability.jlgrows from 72 assertions to 238. It previously covered only scalarcalls with
NoRefractionandDefaultRefraction, so neither the vector path nor anymodel that can widen had coverage.
Other commits
f8ba478is unrelated to the above and rewrites the "making the error controller see theforcing" section of the ModelingToolkit guide. It separates the two fixes the section had
been conflating, and backs the ranking with a measured table against a
reltol = 1e-13reference: declaring the model's own kink times is worth a factor of 125 on the room
temperature at slightly less cost than declaring nothing, whereas the quadrature state is
worth only a factor of two and earns its keep only when the integrand depends on a state.
Happy to split it into its own PR if you would rather review them separately.
ec60e90andd159c46untracktest/.CondaPkgandbenchmark/.CondaPkg, both CondaPkg.jlgenerated environments. Neither is declared by a tracked
CondaPkg.toml, both hold ametabinary cache of absolute paths from whichever machine last resolved them, and both contain
the same Copier template environment of
copier,opensslandlibstdcxx. Notably thatcontent has no
solposxorpandasin it, the two packagesbenchmark/python.jlinstalls,so the tracked environment was not serving the one place that uses CondaPkg either.
docs/.CondaPkgwas already ignored, so the pattern is generalised to**/.CondaPkg/. Thefiles stay on disk and CondaPkg resolves them on demand.
docs/CondaPkg.tomlis adeclaration file and stays tracked.
🤖 Generated with Claude Code