Skip to content

Fix inverted and wrong factors in the units conversion table - #1199

Open
rundvd wants to merge 2 commits into
RocketPy-Team:developfrom
rundvd:bug/units-conversion-factors
Open

rundvd wants to merge 2 commits into
RocketPy-Team:developfrom
rundvd:bug/units-conversion-factors

Conversation

@rundvd

@rundvd rundvd commented Sep 26, 2026

Copy link
Copy Markdown

Pull request type

  • Code changes (bugfix, features)

Checklist

  • Tests for the changes have been added
  • ruff format --check and ruff check pass on the changed files
  • tests/unit passes locally except the Monte Carlo files, which I could not finish in my environment (nothing there uses rocketpy.units); the environment analysis and flight data importer integration tests also pass

Current behavior

Each UNITS_CONVERSION_DICT entry is the number of that unit in one base unit (1 m = 1e3 mm), but six entries do not follow that: deg, grad, mg and g are inverted, ft/s^2 holds the inverse of the correct factor, and atm is 1.01325e-5 instead of 1/101325. convert_units(np.pi, "rad", "deg") returns 0.0548. Because FlightDataImporter divides by these factors, a flight log recorded in ft/s^2 imports 10.8x too large and one recorded in degrees 3283x too large.

New behavior

The six factors are corrected. A parametrized test checks every non-temperature unit against its exact SI definition in both directions, and a new FlightDataImporter test covers ft/s^2 and degree columns.

Breaking change

  • No (results change only for the six affected units)

Additional information

Found and fixed with AI assistance (Claude Opus 5.5); I reviewed the change and the tests fail before it and pass after.

rundvd and others added 2 commits September 25, 2026 20:19
Each UNITS_CONVERSION_DICT entry is the number of that unit in one base
unit (1 m = 1e3 mm). Six entries did not follow that: deg, grad, mg and
g were inverted, ft/s^2 used the inverse of ft/s, and atm was 1.01325e-5
instead of 1/101325. convert_units(np.pi, "rad", "deg") returned 0.0548.

Add a table-driven test that checks every non-temperature unit against
its exact SI definition in both directions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KkQuxkQhvUHzSJbU9Va8a
FlightDataImporter divides imported columns by UNITS_CONVERSION_DICT
entries, so the wrong ft/s^2 and deg factors made a log recorded in
ft/s^2 read 10.8x too large and one recorded in degrees 3283x too large.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KkQuxkQhvUHzSJbU9Va8a
@rundvd
rundvd requested a review from a team as a code owner September 26, 2026 01:35
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.

1 participant