Merge pull request #258 from nyx-space/fix/translate-to

Fix translate_to function to correctly set the frame

.github/workflows/python.yml

@@ -147,11 +147,8 @@ jobs:
           pip install pytest
           pytest
 
-  macos:
-    runs-on: macos-latest
-    strategy:
-      matrix:
-        target: [x86_64, aarch64]
+  macos-13: # last available x86_64 macos runner
+    runs-on: macos-13
     steps:
       - uses: actions/checkout@v4
         with:
@@ -164,7 +161,42 @@ jobs:
       - name: Build wheels
         uses: PyO3/maturin-action@v1
         with:
-          target: ${{ matrix.target }}
+          target: x86_64
+          args: --release --out dist --find-interpreter
+          sccache: 'true'
+          working-directory: anise-py
+
+      - name: Upload wheels
+        uses: actions/upload-artifact@v3
+        with:
+          name: wheels
+          path: anise-py/dist
+
+      - name: pytest
+        shell: bash
+        env:
+          RUST_BACKTRACE: 1
+        run: |
+          set -e
+          pip install anise --find-links anise-py/dist --force-reinstall
+          pip install pytest
+          pytest
+
+  macos-14: # last available x86_64 macos runner
+    runs-on: macos-14
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          lfs: true
+
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.11"
+
+      - name: Build wheels
+        uses: PyO3/maturin-action@v1
+        with:
+          target: aarch64
           args: --release --out dist --find-interpreter
           sccache: 'true'
           working-directory: anise-py
@@ -176,7 +208,6 @@ jobs:
           path: anise-py/dist
 
       - name: pytest
-        if: ${{ !startsWith(matrix.target, 'aarch64') }}
         shell: bash
         env:
           RUST_BACKTRACE: 1
@@ -217,7 +248,7 @@ jobs:
     name: Release
     runs-on: ubuntu-latest
     if: github.ref_type == 'tag'
-    needs: [linux, windows, macos, sdist]
+    needs: [linux, windows, macos-13, macos-14, sdist]
     steps:
       - uses: actions/download-artifact@v3
         with:

anise-py/src/astro.rs

@@ -18,16 +18,19 @@ use anise::frames::Frame;
 
 use super::constants::register_constants;
 
-pub(crate) fn register_astro(py: Python<'_>, parent_module: &PyModule) -> PyResult<()> {
-    let sm = PyModule::new(py, "astro")?;
+pub(crate) fn register_astro(parent_module: &Bound<'_, PyModule>) -> PyResult<()> {
+    let sm = PyModule::new_bound(parent_module.py(), "astro")?;
     sm.add_class::<Ellipsoid>()?;
     sm.add_class::<Frame>()?;
     sm.add_class::<Orbit>()?;
     sm.add_class::<AzElRange>()?;
 
-    register_constants(py, sm)?;
+    register_constants(&sm)?;
 
-    py_run!(py, sm, "import sys; sys.modules['anise.astro'] = sm");
-    parent_module.add_submodule(sm)?;
+    Python::with_gil(|py| {
+        py_run!(py, sm, "import sys; sys.modules['anise.astro'] = sm");
+    });
+
+    parent_module.add_submodule(&sm)?;
     Ok(())
 }

anise-py/src/constants.rs

@@ -185,18 +185,21 @@ impl UsualConstants {
     const SPEED_OF_LIGHT_KM_S: f64 = SPEED_OF_LIGHT_KM_S;
 }
 
-pub(crate) fn register_constants(py: Python<'_>, parent_module: &PyModule) -> PyResult<()> {
-    let sm = PyModule::new(py, "astro.constants")?;
+pub(crate) fn register_constants(parent_module: &Bound<'_, PyModule>) -> PyResult<()> {
+    let sm = PyModule::new_bound(parent_module.py(), "astro.constants")?;
     sm.add_class::<CelestialObjects>()?;
     sm.add_class::<Frames>()?;
     sm.add_class::<Orientations>()?;
     sm.add_class::<UsualConstants>()?;
 
-    py_run!(
-        py,
-        sm,
-        "import sys; sys.modules['anise.astro.constants'] = sm"
-    );
-    parent_module.add_submodule(sm)?;
+    Python::with_gil(|py| {
+        py_run!(
+            py,
+            sm,
+            "import sys; sys.modules['anise.astro.constants'] = sm"
+        );
+    });
+
+    parent_module.add_submodule(&sm)?;
     Ok(())
 }

anise-py/src/lib.rs

@@ -24,10 +24,11 @@ mod utils;
 
 /// A Python module implemented in Rust.
 #[pymodule]
-fn anise(py: Python, m: &PyModule) -> PyResult<()> {
-    register_time_module(py, m)?;
-    astro::register_astro(py, m)?;
-    utils::register_utils(py, m)?;
+fn anise(m: &Bound<'_, PyModule>) -> PyResult<()> {
+    pyo3_log::init();
+    register_time_module(m)?;
+    astro::register_astro(&m)?;
+    utils::register_utils(m)?;
     m.add_class::<Almanac>()?;
     m.add_class::<Aberration>()?;
     m.add_class::<MetaAlmanac>()?;
@@ -36,10 +37,8 @@ fn anise(py: Python, m: &PyModule) -> PyResult<()> {
 }
 
 /// Reexport hifitime as anise.time
-fn register_time_module(py: Python<'_>, parent_module: &PyModule) -> PyResult<()> {
-    pyo3_log::init();
-
-    let sm = PyModule::new(parent_module.py(), "time")?;
+fn register_time_module(parent_module: &Bound<'_, PyModule>) -> PyResult<()> {
+    let sm = PyModule::new_bound(parent_module.py(), "time")?;
 
     sm.add_class::<Epoch>()?;
     sm.add_class::<TimeScale>()?;
@@ -50,7 +49,9 @@ fn register_time_module(py: Python<'_>, parent_module: &PyModule) -> PyResult<()
     sm.add_class::<LeapSecondsFile>()?;
     sm.add_class::<Ut1Provider>()?;
 
-    py_run!(py, sm, "import sys; sys.modules['anise.time'] = sm");
-    parent_module.add_submodule(sm)?;
+    Python::with_gil(|py| {
+        py_run!(py, sm, "import sys; sys.modules['anise.time'] = sm");
+    });
+    parent_module.add_submodule(&sm)?;
     Ok(())
 }

anise-py/src/utils.rs

@@ -15,14 +15,16 @@ use anise::structure::dataset::DataSetError;
 use anise::structure::planetocentric::ellipsoid::Ellipsoid;
 use pyo3::{prelude::*, py_run};
 
-pub(crate) fn register_utils(py: Python<'_>, parent_module: &PyModule) -> PyResult<()> {
-    let sm = PyModule::new(py, "utils")?;
+pub(crate) fn register_utils(parent_module: &Bound<'_, PyModule>) -> PyResult<()> {
+    let sm = PyModule::new_bound(parent_module.py(), "utils")?;
     sm.add_class::<Ellipsoid>()?;
-    sm.add_function(wrap_pyfunction!(convert_fk, sm)?)?;
-    sm.add_function(wrap_pyfunction!(convert_tpc, sm)?)?;
+    sm.add_function(wrap_pyfunction!(convert_fk, &sm)?)?;
+    sm.add_function(wrap_pyfunction!(convert_tpc, &sm)?)?;
 
-    py_run!(py, sm, "import sys; sys.modules['anise.utils'] = sm");
-    parent_module.add_submodule(sm)?;
+    Python::with_gil(|py| {
+        py_run!(py, sm, "import sys; sys.modules['anise.utils'] = sm");
+    });
+    parent_module.add_submodule(&sm)?;
     Ok(())
 }
 

anise/src/almanac/metaload/metaalmanac.rs

@@ -146,7 +146,7 @@ impl MetaAlmanac {
 
     /// Loads the provided string as a Dhall configuration to build a MetaAlmanac
     #[classmethod]
-    fn loads(_cls: &PyType, s: String) -> Result<Self, MetaAlmanacError> {
+    fn loads(_cls: &Bound<'_, PyType>, s: String) -> Result<Self, MetaAlmanacError> {
         Self::from_str(&s)
     }
 
@@ -165,7 +165,7 @@ impl MetaAlmanac {
     /// if queried at some future time, the Earth rotation parameters may have changed between two queries.
     ///
     #[classmethod]
-    fn latest(_cls: &PyType, py: Python) -> AlmanacResult<Almanac> {
+    fn latest(_cls: &Bound<'_, PyType>, py: Python) -> AlmanacResult<Almanac> {
         let mut meta = Self::default();
         py.allow_threads(|| match meta._process() {
             Ok(almanac) => Ok(almanac),

anise/src/astro/orbit.rs

@@ -479,7 +479,7 @@ impl Orbit {
     #[cfg(feature = "python")]
     #[classmethod]
     pub fn from_keplerian(
-        _cls: &PyType,
+        _cls: &Bound<'_, PyType>,
         sma: f64,
         ecc: f64,
         inc: f64,
@@ -496,7 +496,7 @@ impl Orbit {
     #[cfg(feature = "python")]
     #[classmethod]
     pub fn from_keplerian_apsis_radii(
-        _cls: &PyType,
+        _cls: &Bound<'_, PyType>,
         r_a: f64,
         r_p: f64,
         inc: f64,
@@ -518,7 +518,7 @@ impl Orbit {
     #[cfg(feature = "python")]
     #[classmethod]
     pub fn from_keplerian_mean_anomaly(
-        _cls: &PyType,
+        _cls: &Bound<'_, PyType>,
         sma_km: f64,
         ecc: f64,
         inc_deg: f64,

anise/src/astro/orbit_geodetic.rs

@@ -119,7 +119,7 @@ impl CartesianState {
     #[cfg(feature = "python")]
     #[classmethod]
     pub fn from_keplerian_altitude(
-        _cls: &PyType,
+        _cls: &Bound<'_, PyType>,
         sma_altitude: f64,
         ecc: f64,
         inc: f64,
@@ -137,7 +137,7 @@ impl CartesianState {
     #[cfg(feature = "python")]
     #[classmethod]
     pub fn from_keplerian_apsis_altitude(
-        _cls: &PyType,
+        _cls: &Bound<'_, PyType>,
         apo_alt: f64,
         peri_alt: f64,
         inc: f64,
@@ -158,7 +158,7 @@ impl CartesianState {
     #[cfg(feature = "python")]
     #[classmethod]
     pub fn from_latlongalt(
-        _cls: &PyType,
+        _cls: &Bound<'_, PyType>,
         latitude_deg: f64,
         longitude_deg: f64,
         height_km: f64,

anise/src/ephemerides/translations.rs

@@ -49,7 +49,7 @@ impl Almanac {
     pub fn translate(
         &self,
         target_frame: Frame,
-        observer_frame: Frame,
+        mut observer_frame: Frame,
         epoch: Epoch,
         ab_corr: Option<Aberration>,
     ) -> Result<CartesianState, EphemerisError> {
@@ -58,12 +58,10 @@ impl Almanac {
             return Ok(CartesianState::zero(observer_frame));
         }
 
-        let mut obs_frame: Frame = observer_frame;
-
         // If there is no frame info, the user hasn't loaded this frame, but might still want to compute a translation.
-        if let Ok(obs_frame_info) = self.frame_from_uid(obs_frame) {
+        if let Ok(obs_frame_info) = self.frame_from_uid(observer_frame) {
             // User has loaded the planetary data for this frame, so let's use that as the to_frame.
-            obs_frame = obs_frame_info;
+            observer_frame = obs_frame_info;
         }
 
         match ab_corr {
@@ -116,7 +114,7 @@ impl Almanac {
                     radius_km: pos_bwrd - pos_fwrd,
                     velocity_km_s: vel_bwrd - vel_fwrd,
                     epoch,
-                    frame: obs_frame.with_orient(target_frame.orientation_id),
+                    frame: observer_frame.with_orient(target_frame.orientation_id),
                 })
             }
             Some(ab_corr) => {
@@ -193,12 +191,19 @@ impl Almanac {
     pub fn translate_to(
         &self,
         state: CartesianState,
-        observer_frame: Frame,
+        mut observer_frame: Frame,
         ab_corr: Option<Aberration>,
     ) -> Result<CartesianState, EphemerisError> {
         let frame_state = self.translate(state.frame, observer_frame, state.epoch, ab_corr)?;
+        let mut new_state = state.add_unchecked(&frame_state);
 
-        Ok(state.add_unchecked(&frame_state))
+        // If there is no frame info, the user hasn't loaded this frame, but might still want to compute a translation.
+        if let Ok(obs_frame_info) = self.frame_from_uid(observer_frame) {
+            // User has loaded the planetary data for this frame, so let's use that as the to_frame.
+            observer_frame = obs_frame_info;
+        }
+        new_state.frame = observer_frame.with_orient(state.frame.orientation_id);
+        Ok(new_state)
     }
 }
 

anise/src/math/cartesian.rs

@@ -223,7 +223,8 @@ impl CartesianState {
     /// Returns the distance in kilometers between this state and another state, if both frame match (epoch does not need to match).
     pub fn distance_to_km(&self, other: &Self) -> PhysicsResult<f64> {
         ensure!(
-            self.frame == other.frame,
+            self.frame.ephem_origin_match(other.frame)
+                && self.frame.orient_origin_match(other.frame),
             FrameMismatchSnafu {
                 action: "computing distance between states",
                 frame1: self.frame,
@@ -237,7 +238,8 @@ impl CartesianState {
     /// Returns the root mean squared (RSS) radius difference between this state and another state, if both frames match (epoch does not need to match)
     pub fn rss_radius_km(&self, other: &Self) -> PhysicsResult<f64> {
         ensure!(
-            self.frame == other.frame,
+            self.frame.ephem_origin_match(other.frame)
+                && self.frame.orient_origin_match(other.frame),
             FrameMismatchSnafu {
                 action: "computing radius RSS",
                 frame1: self.frame,
@@ -250,7 +252,8 @@ impl CartesianState {
     /// Returns the root mean squared (RSS) velocity difference between this state and another state, if both frames match (epoch does not need to match)
     pub fn rss_velocity_km_s(&self, other: &Self) -> PhysicsResult<f64> {
         ensure!(
-            self.frame == other.frame,
+            self.frame.ephem_origin_match(other.frame)
+                && self.frame.orient_origin_match(other.frame),
             FrameMismatchSnafu {
                 action: "computing velocity RSS",
                 frame1: self.frame,
@@ -269,7 +272,8 @@ impl CartesianState {
             && (self.velocity_km_s.x - other.velocity_km_s.x).abs() < velocity_tol_km_s
             && (self.velocity_km_s.y - other.velocity_km_s.y).abs() < velocity_tol_km_s
             && (self.velocity_km_s.z - other.velocity_km_s.z).abs() < velocity_tol_km_s
-            && self.frame == other.frame
+            && self.frame.ephem_origin_match(other.frame)
+            && self.frame.orient_origin_match(other.frame)
     }
 
     /// Returns the light time duration between this object and the origin of its reference frame.

anise/tests/almanac/mod.rs

@@ -56,6 +56,13 @@ fn test_state_transformation() {
         .transform_to(orig_state, EARTH_ITRF93, Aberration::NONE)
         .unwrap();
 
+    // Check that the frame is correctly set.
+    assert_eq!(state_itrf93.frame.ephemeris_id, EARTH_ITRF93.ephemeris_id);
+    assert_eq!(
+        state_itrf93.frame.orientation_id,
+        EARTH_ITRF93.orientation_id
+    );
+
     println!("{orig_state:x}");
     println!("{state_itrf93:X}");
 
@@ -66,6 +73,7 @@ fn test_state_transformation() {
         .unwrap();
 
     println!("{from_state_itrf93_to_eme2k}");
+    println!("{from_state_itrf93_to_eme2k:x}");
 
     assert_eq!(orig_state, from_state_itrf93_to_eme2k);
 }