[#144] Correct noncircular orbit generation

Author: Mark2000

docs/source/release_notes.rst

@@ -38,6 +38,9 @@ Development - |version|
 * Allow for per-episode randomization of :class:`ResourceReward` weights and observation
   of those weights with :class:`ResourceRewardWeight`.
 * Add :class:`ImpulsiveThrustHill` for impulsive thrust in the Hill frame.
+* Separate :class:`random_circular_orbit` and :class:`random_orbit` to avoid misleading
+  altitude argument.
+
 
 Version 1.1.0
 -------------

src/bsk_rl/utils/orbital.py

@@ -21,33 +21,48 @@
 
 
 def random_orbit(
-    i: Optional[float] = 45.0,
-    alt: float = 500,
-    r_body: float = 6371,
+    i: Optional[float] = None,
+    a: Optional[float] = 6371 + 500,
     e: float = 0,
     Omega: Optional[float] = None,
-    omega: Optional[float] = 0,
+    omega: Optional[float] = None,
     f: Optional[float] = None,
+    alt: float = None,
+    r_body: float = 6371,
 ) -> ClassicElements:
     """Create a set of orbit elements.
 
     Parameters are fixed if specified and randomized if ``None``. Defaults to a random
-    circular orbit at 500 km altitude and 45 deg inclination.
+    circular orbit at 500 km altitude.
 
     Args:
         i: [deg] Inclination, randomized in ``[-pi, pi]``.
-        alt: [km] Altitude above r_body.
-        r_body: [km] Body radius.
+        a: [km] Semi-major axis.
         e: Eccentricity.
         Omega: [deg] LAN, randomized in ``[0, 2pi]``.
         omega: [deg] Argument of periapsis, randomized in ``[0, 2pi]``.
         f: [deg] True anomaly, randomized in ``[0, 2pi]``.
+        alt: [km] (deprecated) Altitude above r_body.
+        r_body: [km] (deprecated) Body radius.
 
     Returns:
         ClassicElements: orbital elements
     """
+    if alt is not None:
+        logger.warning(
+            "Ignoring a, e, and omega and using alt and r_body to generate a circular orbit."
+            "random_circular_orbit is preferred for this use case."
+        )
+        return random_circular_orbit(
+            i=i,
+            alt=alt,
+            r_body=r_body,
+            Omega=Omega,
+            f=f,
+        )
+
     oe = ClassicElements()
-    oe.a = (r_body + alt) * 1e3
+    oe.a = a * 1e3
     oe.e = e
     oe.i = np.radians(i) if i is not None else np.random.uniform(-np.pi, np.pi)
     oe.Omega = (
@@ -60,6 +75,32 @@ def random_orbit(
     return oe
 
 
+def random_circular_orbit(
+    i: Optional[float] = None,
+    alt: float = 500,
+    r_body: float = 6371,
+    Omega: Optional[float] = None,
+    f: Optional[float] = None,
+):
+    """Create a set of orbit elements for a circular orbit.
+
+    Parameters are fixed if specified and randomized if ``None``.
+
+    Args:
+        i: [deg] Inclination, randomized in ``[-pi, pi]``.
+        alt: [km] Altitude above r_body.
+        r_body: [km] Body radius, defaults to Earth's radius.
+        Omega: [deg] LAN, randomized in ``[0, 2pi]``.
+        f: [deg] True anomaly, randomized in ``[0, 2pi]``.
+
+    """
+    omega = 0
+    e = 0
+    a = r_body + alt
+
+    return random_orbit(i=i, a=a, e=e, Omega=Omega, omega=omega, f=f)
+
+
 def random_epoch(start: int = 2000, end: int = 2022):
     """Generate a random epoch in a year range.
 
@@ -616,6 +657,7 @@ def hill2cd(
 __doc_title__ = "Orbital"
 __all__ = [
     "random_orbit",
+    "random_circular_orbit",
     "random_epoch",
     "lla2ecef",
     "elevation",

tests/integration/act/test_int_actions_discrete.py

@@ -86,7 +86,7 @@ class ChargeSat(sats.Satellite):
             "Charger",
             sat_args=ChargeSat.default_sat_args(
                 # High, inclined orbit makes eclipse unlikely
-                oe=random_orbit(alt=50000, i=90),
+                oe=random_orbit(a=50000, i=90),
                 batteryStorageCapacity=500_000,
                 storedCharge_Init=250_000,
             ),

tests/integration/obs/test_int_observations.py

@@ -71,9 +71,7 @@ class SatPropertiesSat(sats.Satellite):
         "SatelliteTasking-v1",
         satellite=SatPropertiesSat(
             "Sputnik",
-            sat_args=SatPropertiesSat.default_sat_args(
-                oe=random_orbit(r_body=7000, alt=0, i=45)
-            ),
+            sat_args=SatPropertiesSat.default_sat_args(oe=random_orbit(a=7000, i=45)),
         ),
         scenario=UniformTargets(n_targets=0),
         rewarder=data.NoReward(),

tests/integration/obs/test_int_relative_observations.py

@@ -28,17 +28,13 @@ class RelPropertiesSat(sats.Satellite):
             RelPropertiesSat(
                 "Deputy",
                 sat_args=dict(
-                    oe=random_orbit(
-                        i=1.0, alt=1000.0, e=0.0, Omega=0.0, omega=0.0, f=0.0
-                    )
+                    oe=random_orbit(i=1.0, a=7000.0, e=0.0, Omega=0.0, omega=0.0, f=0.0)
                 ),
             ),
             BasicSat(
                 "Chief",
                 sat_args=dict(
-                    oe=random_orbit(
-                        i=1.0, alt=1100.0, e=0.0, Omega=0.0, omega=0.0, f=0.0
-                    )
+                    oe=random_orbit(i=1.0, a=7100.0, e=0.0, Omega=0.0, omega=0.0, f=0.0)
                 ),
             ),
         ],

tests/unittest/utils/test_orbital.py

@@ -17,6 +17,13 @@ def test_random_orbit(self):
         assert 0 <= oe.omega <= 2 * np.pi
         assert 0 <= oe.f <= 2 * np.pi
 
+    @pytest.mark.repeat(10)
+    def test_random_circular_orbit(self):
+        oe = orbital.random_circular_orbit(i=None, Omega=None, f=None)
+        assert -np.pi <= oe.i <= np.pi
+        assert 0 <= oe.Omega <= 2 * np.pi
+        assert 0 <= oe.f <= 2 * np.pi
+
     def test_repeatable(self):
         np.random.seed(0)
         oe1 = orbital.random_orbit()
@@ -25,9 +32,7 @@ def test_repeatable(self):
         assert oe1.f == oe2.f
 
     def test_units(self):
-        oe = orbital.random_orbit(
-            i=90.0, alt=500, r_body=1000, e=0.1, Omega=90.0, omega=90.0, f=90.0
-        )
+        oe = orbital.random_orbit(i=90.0, a=1500, e=0.1, Omega=90.0, omega=90.0, f=90.0)
         assert oe.a == 1500000
         assert oe.e == 0.1
         assert np.pi / 2 == oe.i == oe.Omega == oe.omega == oe.f
@@ -198,7 +203,7 @@ def test_eclipse(self):
     def test_no_eclipse(self):
         ts = orbital.TrajectorySimulator(
             self.epoch,
-            oe=orbital.random_orbit(alt=50000, i=90.0, omega=0, Omega=0, f=45.0),
+            oe=orbital.random_orbit(a=50000, i=90.0, omega=0, Omega=0, f=45.0),
             mu=self.mu,
         )
         assert ts.next_eclipse(0, max_tries=3) == (1.0, 1.0)