From eb48c3af9a8f0e46b959bbeec7ccedf4189f6534 Mon Sep 17 00:00:00 2001 From: David Rundus <114828622+rundvd@users.noreply.github.com> Date: Sat, 26 Sep 2026 01:00:08 +0000 Subject: [PATCH 1/2] BUG: fix inverted and wrong factors in the units conversion table 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 Claude-Session: https://claude.ai/code/session_017KkQuxkQhvUHzSJbU9Va8a --- rocketpy/units.py | 12 +++++----- tests/unit/test_units.py | 50 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/rocketpy/units.py b/rocketpy/units.py index b8e4c199c..f044c2782 100644 --- a/rocketpy/units.py +++ b/rocketpy/units.py @@ -25,14 +25,14 @@ # Units of acceleration. Meter per square second "m/s^2" is the base unit. "m/s^2": 1, "gs": 1 / 9.80665, - "ft/s^2": 1 / 3.2808399, + "ft/s^2": 1 / 0.3048, # Units of pressure. Pascal "Pa" is the base unit. "Pa": 1, "hPa": 1e-2, "kPa": 1e-3, "MPa": 1e-6, "bar": 1e-5, - "atm": 1.01325e-5, + "atm": 1 / 101325, "mmHg": 1 / 133.322, "inHg": 1 / 3386.389, # Units of time. Seconds "s" is the base unit. @@ -41,14 +41,14 @@ "h": 1 / 3600, "d": 1 / 86400, # Units of mass. Kilogram "kg" is the base unit. - "mg": 1e-6, - "g": 1e-3, + "mg": 1e6, + "g": 1e3, "kg": 1, "lb": 2.20462, # Units of angle. Radian "rad" is the base unit. "rad": 1, - "deg": 1 / 180 * np.pi, - "grad": 1 / 200 * np.pi, + "deg": 180 / np.pi, + "grad": 200 / np.pi, } diff --git a/tests/unit/test_units.py b/tests/unit/test_units.py index 8c3db1d00..f63c808c3 100644 --- a/tests/unit/test_units.py +++ b/tests/unit/test_units.py @@ -68,6 +68,56 @@ def test_conversion_factor_invalid_conversion(self): class TestConvertUnits: """Tests for the convert_units function.""" + @pytest.mark.parametrize( + "unit, base_unit, value_in_base", + [ + ("mm", "m", 1e-3), + ("cm", "m", 1e-2), + ("dm", "m", 1e-1), + ("dam", "m", 1e1), + ("hm", "m", 1e2), + ("km", "m", 1e3), + ("ft", "m", 0.3048), + ("in", "m", 0.0254), + ("mi", "m", 1609.344), + ("nmi", "m", 1852), + ("yd", "m", 0.9144), + ("km/h", "m/s", 1 / 3.6), + ("knot", "m/s", 1852 / 3600), + ("mph", "m/s", 1609.344 / 3600), + ("ft/s", "m/s", 0.3048), + ("gs", "m/s^2", 9.80665), + ("ft/s^2", "m/s^2", 0.3048), + ("hPa", "Pa", 1e2), + ("kPa", "Pa", 1e3), + ("MPa", "Pa", 1e6), + ("bar", "Pa", 1e5), + ("atm", "Pa", 101325), + ("mmHg", "Pa", 133.322387415), + ("inHg", "Pa", 3386.389), + ("min", "s", 60), + ("h", "s", 3600), + ("d", "s", 86400), + ("mg", "kg", 1e-6), + ("g", "kg", 1e-3), + ("lb", "kg", 0.45359237), + ("deg", "rad", np.pi / 180), + ("grad", "rad", np.pi / 200), + ], + ) + def test_convert_units_matches_unit_definitions( + self, unit, base_unit, value_in_base + ): + """One of each unit should convert to its defined value in the base + unit, and back. The references are the exact SI definitions, so the + tolerance only allows for the rounded pound and mercury constants.""" + assert convert_units(1, unit, base_unit) == pytest.approx( + value_in_base, rel=1e-5 + ) + assert convert_units(value_in_base, base_unit, unit) == pytest.approx( + 1, rel=1e-5 + ) + def test_convert_units_same_unit(self): assert convert_units(300, "K", "K") == 300 assert convert_units(27, "degC", "degC") == 27 From 6e862389cd1b1b27480626654c87ed0d01aec9b1 Mon Sep 17 00:00:00 2001 From: David Rundus <114828622+rundvd@users.noreply.github.com> Date: Sat, 26 Sep 2026 01:01:58 +0000 Subject: [PATCH 2/2] TST: check FlightDataImporter converts ft/s^2 and degree columns 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 Claude-Session: https://claude.ai/code/session_017KkQuxkQhvUHzSJbU9Va8a --- .../simulation/test_flight_data_importer.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/integration/simulation/test_flight_data_importer.py b/tests/integration/simulation/test_flight_data_importer.py index 9cff30d57..653222ed1 100644 --- a/tests/integration/simulation/test_flight_data_importer.py +++ b/tests/integration/simulation/test_flight_data_importer.py @@ -55,3 +55,22 @@ def test_flight_importer_ndrt(): "Can't find 'altitude' column in fd._columns" ) assert np.isclose(fd.altitude(0), 0) + + +def test_flight_importer_converts_imperial_and_degree_columns(tmp_path): + """Columns logged in ft/s^2 and degrees should be converted to SI.""" + path = tmp_path / "log.csv" + path.write_text("time,accel_ft_s2,pitch_deg\n0,32.174,90\n1,32.174,90\n") + + fd = FlightDataImporter( + paths=str(path), + columns_map={ + "time": "time", + "accel_ft_s2": "az", + "pitch_deg": "attitude_angle", + }, + units={"accel_ft_s2": "ft/s^2", "pitch_deg": "deg"}, + ) + + assert np.isclose(fd.az(0), 32.174 * 0.3048) + assert np.isclose(fd.attitude_angle(0), np.pi / 2)