Skip to content

density.nrlmsise: float overload takes radians as documented (was passing them through as degrees) - #171

Merged
ssmichael1 merged 2 commits into
mainfrom
fix/density-nrlmsise-radians
Aug 31, 2026
Merged

density.nrlmsise: float overload takes radians as documented (was passing them through as degrees)#171
ssmichael1 merged 2 commits into
mainfrom
fix/density-nrlmsise-radians

Conversation

@ssmichael1

Copy link
Copy Markdown
Owner

satkit.density.nrlmsise(altitude_m, latitude_rad, longitude_rad, time) is documented in radians (satkit's convention without a _deg suffix) but handed the two floats unchanged to nrlmsise::nrlmsise, which takes degrees — the slot the itrfcoord overload fills with latitude_deg() (fixed in #154) and satkit.nrlmsise00(latitude_deg=...) fills explicitly. A caller following the stub at 60° N was evaluated at 1.05° N. Found by the #90 units sweep (#169).

Fix is a .to_degrees() on each value in the float branch of python/src/pydensity.rs; stub and docs were already right. New test test_density_float_form_takes_radians checks the float form with radians lands on the same density and temperature as the itrfcoord form at the same point (rel 1e-6).

Closes #170

🤖 Generated with Claude Code

https://claude.ai/code/session_01PG969yapJ84DJceKt21Wen

ssmichael1 added a commit that referenced this pull request Aug 31, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PG969yapJ84DJceKt21Wen
@ssmichael1 ssmichael1 closed this Aug 31, 2026
@ssmichael1 ssmichael1 reopened this Aug 31, 2026
ssmichael1 and others added 2 commits August 30, 2026 20:56
…de to the degrees the model takes

The stub documents latitude_rad / longitude_rad; the binding passed the
values through unchanged to nrlmsise::nrlmsise, which takes degrees (the
itrfcoord overload and nrlmsise00(latitude_deg=...) already supply
degrees). A caller following the stub was evaluated at ~1 deg latitude
instead of 60 deg. Regression test: the float form with radians matches
the itrfcoord form at the same point.

Closes #170

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PG969yapJ84DJceKt21Wen
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PG969yapJ84DJceKt21Wen
@ssmichael1
ssmichael1 force-pushed the fix/density-nrlmsise-radians branch from 250d6b4 to 18cbcfc Compare August 31, 2026 00:57
@ssmichael1
ssmichael1 merged commit e5e3cab into main Aug 31, 2026
8 checks passed
@ssmichael1
ssmichael1 deleted the fix/density-nrlmsise-radians branch August 31, 2026 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

density.nrlmsise(altitude, latitude, longitude, time) passes radians through as degrees

1 participant