From df3dab1ae2e1de9d5a5c97d185be839008ccb6bd Mon Sep 17 00:00:00 2001 From: Dar Dahlen Date: Wed, 8 Jan 2025 10:18:22 -0800 Subject: [PATCH] Change SPICE kernel singleton structure (#167) * Change SPICE kernel singleton structure * doc test fix --- CHANGELOG.md | 1 + src/kete/rust/propagation.rs | 19 ++++--- src/kete/rust/spice/pck.rs | 12 ++--- src/kete/rust/spice/spk.rs | 14 ++--- src/kete/rust/state.rs | 2 +- src/kete_core/benches/propagation.rs | 2 +- src/kete_core/benches/spice.rs | 10 ++-- src/kete_core/src/fov/fov_like.rs | 2 +- src/kete_core/src/fov/generic.rs | 2 +- src/kete_core/src/lib.rs | 2 +- src/kete_core/src/propagation/acceleration.rs | 6 +-- src/kete_core/src/propagation/mod.rs | 6 +-- src/kete_core/src/spice/pck.rs | 38 +++++-------- src/kete_core/src/spice/spk.rs | 53 +++++++------------ src/kete_core/src/spice/spk_segments.rs | 4 +- 15 files changed, 71 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67e6de7..fb74b86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Comet Magnitude estimates now accepts two phase correction values instead of 1. +- Restructured SPICE kernel memory management to make entire class of bugs impossible. ### Fixed diff --git a/src/kete/rust/propagation.rs b/src/kete/rust/propagation.rs index 0974f2c..d749122 100644 --- a/src/kete/rust/propagation.rs +++ b/src/kete/rust/propagation.rs @@ -3,7 +3,7 @@ use itertools::Itertools; use kete_core::{ errors::Error, propagation::{self, moid, NonGravModel}, - spice::{self, get_spk_singleton}, + spice::{self, LOADED_SPK}, state::State, time::{scales::TDB, Time}, }; @@ -27,15 +27,14 @@ use crate::{nongrav::PyNonGravModel, time::PyTime}; #[pyfunction] #[pyo3(name = "moid", signature = (state_a, state_b=None))] pub fn moid_py(state_a: PyState, state_b: Option) -> PyResult { - let state_b = - state_b - .map(|x| x.0) - .unwrap_or(get_spk_singleton().read().unwrap().try_get_state( - 399, - state_a.0.jd, - 10, - state_a.0.frame, - )?); + let state_b = state_b + .map(|x| x.0) + .unwrap_or(LOADED_SPK.read().unwrap().try_get_state( + 399, + state_a.0.jd, + 10, + state_a.0.frame, + )?); Ok(moid(state_a.0, state_b)?) } diff --git a/src/kete/rust/spice/pck.rs b/src/kete/rust/spice/pck.rs index 0e1178f..004246b 100644 --- a/src/kete/rust/spice/pck.rs +++ b/src/kete/rust/spice/pck.rs @@ -1,5 +1,5 @@ use kete_core::frames::ecef_to_geodetic_lat_lon; -use kete_core::spice::{get_pck_singleton, get_spk_singleton}; +use kete_core::spice::{LOADED_PCK, LOADED_SPK}; use kete_core::{constants, prelude::*}; use pyo3::{pyfunction, PyResult}; @@ -9,7 +9,7 @@ use crate::state::PyState; #[pyfunction] #[pyo3(name = "pck_load")] pub fn pck_load_py(filenames: Vec) -> PyResult<()> { - let mut singleton = get_pck_singleton().write().unwrap(); + let mut singleton = LOADED_PCK.write().unwrap(); for filename in filenames.iter() { let load = (*singleton).load_file(filename); if let Err(err) = load { @@ -52,13 +52,13 @@ pub fn pck_earth_frame_py( None => Desig::Empty, } }; - let pcks = get_pck_singleton().try_read().unwrap(); + let pcks = LOADED_PCK.try_read().unwrap(); let frame = pcks.try_get_orientation(3000, jd)?; let mut state = State::new(desig, jd, pos.into(), [0.0, 0.0, 0.0].into(), frame, 399); state.try_change_frame_mut(Frame::Ecliptic)?; - let spks = get_spk_singleton().try_read().unwrap(); + let spks = &LOADED_SPK.try_read().unwrap(); spks.try_change_center(&mut state, new_center)?; Ok(PyState(state)) } @@ -78,7 +78,7 @@ pub fn pck_earth_frame_py( #[pyfunction] #[pyo3(name = "state_to_earth_pos")] pub fn pck_state_to_earth(state: PyState) -> PyResult<(f64, f64, f64)> { - let pcks = get_pck_singleton().try_read().unwrap(); + let pcks = LOADED_PCK.try_read().unwrap(); let state = state.change_center(399)?.as_ecliptic()?; let frame = pcks.try_get_orientation(3000, state.jd())?; let mut state = state.0; @@ -98,5 +98,5 @@ pub fn pck_state_to_earth(state: PyState) -> PyResult<(f64, f64, f64)> { #[pyfunction] #[pyo3(name = "pck_reset")] pub fn pck_reset_py() { - get_pck_singleton().write().unwrap().reset() + LOADED_PCK.write().unwrap().reset() } diff --git a/src/kete/rust/spice/spk.rs b/src/kete/rust/spice/spk.rs index 8d84cdf..7b39dd6 100644 --- a/src/kete/rust/spice/spk.rs +++ b/src/kete/rust/spice/spk.rs @@ -1,4 +1,4 @@ -use kete_core::spice::{get_spk_singleton, try_name_from_id}; +use kete_core::spice::{try_name_from_id, LOADED_SPK}; use pyo3::{pyfunction, PyResult, Python}; use crate::frame::PyFrames; @@ -9,7 +9,7 @@ use crate::time::PyTime; #[pyfunction] #[pyo3(name = "spk_load")] pub fn spk_load_py(py: Python<'_>, filenames: Vec) -> PyResult<()> { - let mut singleton = get_spk_singleton().write().unwrap(); + let mut singleton = LOADED_SPK.write().unwrap(); if filenames.len() > 100 { eprintln!("Loading {} spk files...", filenames.len()); } @@ -30,7 +30,7 @@ pub fn spk_load_py(py: Python<'_>, filenames: Vec) -> PyResult<()> { #[pyfunction] #[pyo3(name = "spk_available_info")] pub fn spk_available_info_py(naif_id: i64) -> Vec<(f64, f64, i64, PyFrames, i32)> { - let singleton = get_spk_singleton().try_read().unwrap(); + let singleton = &LOADED_SPK.try_read().unwrap(); singleton .available_info(naif_id) .into_iter() @@ -42,7 +42,7 @@ pub fn spk_available_info_py(naif_id: i64) -> Vec<(f64, f64, i64, PyFrames, i32) #[pyfunction] #[pyo3(name = "spk_loaded")] pub fn spk_loaded_objects_py() -> Vec { - let spk = get_spk_singleton().try_read().unwrap(); + let spk = &LOADED_SPK.try_read().unwrap(); let loaded = spk.loaded_objects(false); let mut loaded: Vec = loaded.into_iter().collect(); loaded.sort(); @@ -61,7 +61,7 @@ pub fn spk_get_name_from_id_py(id: i64) -> String { #[pyfunction] #[pyo3(name = "spk_reset")] pub fn spk_reset_py() { - get_spk_singleton().write().unwrap().reset() + LOADED_SPK.write().unwrap().reset() } /// Calculate the state of a given object in the target frame. @@ -82,7 +82,7 @@ pub fn spk_reset_py() { #[pyo3(name = "spk_state")] pub fn spk_state_py(id: i64, jd: PyTime, center: i64, frame: PyFrames) -> PyResult { let jd = jd.jd(); - let spk = get_spk_singleton().try_read().unwrap(); + let spk = &LOADED_SPK.try_read().unwrap(); let mut state = spk.try_get_state(id, jd, center, frame.into())?; let _ = state.try_naif_id_to_name(); Ok(PyState(state)) @@ -102,6 +102,6 @@ pub fn spk_state_py(id: i64, jd: PyTime, center: i64, frame: PyFrames) -> PyResu #[pyo3(name = "spk_raw_state")] pub fn spk_raw_state_py(id: i64, jd: PyTime) -> PyResult { let jd = jd.jd(); - let spk = get_spk_singleton().try_read().unwrap(); + let spk = &LOADED_SPK.try_read().unwrap(); Ok(PyState(spk.try_get_raw_state(id, jd)?)) } diff --git a/src/kete/rust/state.rs b/src/kete/rust/state.rs index a79cbd4..ed569ad 100644 --- a/src/kete/rust/state.rs +++ b/src/kete/rust/state.rs @@ -88,7 +88,7 @@ impl PyState { /// If the desired state is not a known NAIF id this will raise an exception. pub fn change_center(&self, naif_id: i64) -> PyResult { let mut state = self.0.clone(); - let spk = prelude::get_spk_singleton().try_read().unwrap(); + let spk = prelude::LOADED_SPK.try_read().unwrap(); spk.try_change_center(&mut state, naif_id)?; Ok(Self(state)) } diff --git a/src/kete_core/benches/propagation.rs b/src/kete_core/benches/propagation.rs index 18a4639..44bd050 100644 --- a/src/kete_core/benches/propagation.rs +++ b/src/kete_core/benches/propagation.rs @@ -57,7 +57,7 @@ fn prop_n_body_radau(state: State, dt: f64) { } fn prop_n_body_vec_radau(mut state: State, dt: f64) { - let spk = get_spk_singleton().read().unwrap(); + let spk = &LOADED_SPK.read().unwrap(); spk.try_change_center(&mut state, 10).unwrap(); state.try_change_frame_mut(Frame::Ecliptic).unwrap(); let states = vec![state.clone(); 100]; diff --git a/src/kete_core/benches/spice.rs b/src/kete_core/benches/spice.rs index b1cd168..e0af66c 100644 --- a/src/kete_core/benches/spice.rs +++ b/src/kete_core/benches/spice.rs @@ -1,17 +1,17 @@ extern crate criterion; use criterion::{black_box, criterion_group, criterion_main, Criterion}; -use kete_core::{spice::get_spk_singleton, state::State}; +use kete_core::{spice::LOADED_SPK, state::State}; use pprof::criterion::{Output, PProfProfiler}; fn spice_get_raw_state(jd: f64) { - let spice = get_spk_singleton().try_read().unwrap(); + let spice = &LOADED_SPK.try_read().unwrap(); for _ in 0..1000 { let _ = spice.try_get_raw_state(5, jd).unwrap(); } } fn spice_change_center(mut state: State) { - let spice = get_spk_singleton().try_read().unwrap(); + let spice = &LOADED_SPK.try_read().unwrap(); for _ in 0..500 { spice.try_change_center(&mut state, 10).unwrap(); spice.try_change_center(&mut state, 0).unwrap(); @@ -19,7 +19,7 @@ fn spice_change_center(mut state: State) { } fn spice_get_state(jd: f64) { - let spice = get_spk_singleton().try_read().unwrap(); + let spice = &LOADED_SPK.try_read().unwrap(); for _ in 0..1000 { let _ = spice .try_get_state(5, jd, 10, kete_core::frames::Frame::Ecliptic) @@ -28,7 +28,7 @@ fn spice_get_state(jd: f64) { } pub fn spice_benchmark(c: &mut Criterion) { - let spice = get_spk_singleton().try_read().unwrap(); + let spice = &LOADED_SPK.try_read().unwrap(); let state = spice .try_get_state(5, 2451545.0, 10, kete_core::frames::Frame::Ecliptic) .unwrap(); diff --git a/src/kete_core/src/fov/fov_like.rs b/src/kete_core/src/fov/fov_like.rs index bd8f4be..ac4ee57 100644 --- a/src/kete_core/src/fov/fov_like.rs +++ b/src/kete_core/src/fov/fov_like.rs @@ -195,7 +195,7 @@ pub trait FovLike: Sync + Sized { /// This will fail silently if the object is not found. fn check_spks(&self, obj_ids: &[i64]) -> Vec> { let obs = self.observer(); - let spk = get_spk_singleton().try_read().unwrap(); + let spk = &LOADED_SPK.try_read().unwrap(); let mut visible: Vec> = vec![Vec::new(); self.n_patches()]; diff --git a/src/kete_core/src/fov/generic.rs b/src/kete_core/src/fov/generic.rs index cee6ed0..29acc7b 100644 --- a/src/kete_core/src/fov/generic.rs +++ b/src/kete_core/src/fov/generic.rs @@ -243,7 +243,7 @@ mod tests { fn test_check_omni_visible() { // Build an observer, and check the observability of ceres with different offsets from the observer time. // this will exercise the position, velocity, and time offsets due to light delay. - let spk = get_spk_singleton().read().unwrap(); + let spk = &LOADED_SPK.read().unwrap(); let observer = State::new( Desig::Empty, 2451545.0, diff --git a/src/kete_core/src/lib.rs b/src/kete_core/src/lib.rs index a6b7850..9be1573 100644 --- a/src/kete_core/src/lib.rs +++ b/src/kete_core/src/lib.rs @@ -56,6 +56,6 @@ pub mod prelude { pub use crate::frames::Frame; pub use crate::propagation::{propagate_n_body_spk, propagate_two_body}; pub use crate::simult_states::SimultaneousStates; - pub use crate::spice::{get_pck_singleton, get_spk_singleton}; + pub use crate::spice::{LOADED_PCK, LOADED_SPK}; pub use crate::state::{Desig, State}; } diff --git a/src/kete_core/src/propagation/acceleration.rs b/src/kete_core/src/propagation/acceleration.rs index bbe29ec..3ebef64 100644 --- a/src/kete_core/src/propagation/acceleration.rs +++ b/src/kete_core/src/propagation/acceleration.rs @@ -19,7 +19,7 @@ //! information should be recorded. //! use crate::prelude::KeteResult; -use crate::spice::get_spk_singleton; +use crate::spice::LOADED_SPK; use crate::{constants::*, errors::Error, frames::Frame, propagation::nongrav::NonGravModel}; use nalgebra::allocator::Allocator; use nalgebra::{DefaultAllocator, Dim, Matrix3, OVector, Vector3, U1, U2}; @@ -135,7 +135,7 @@ pub fn spk_accel( } } - let spk = get_spk_singleton().try_read().unwrap(); + let spk = &LOADED_SPK.try_read().unwrap(); for grav_params in meta.massive_obj.iter() { let id = grav_params.naif_id; @@ -297,7 +297,7 @@ mod tests { #[test] fn check_accelerations_equal() { - let spk = get_spk_singleton().try_read().unwrap(); + let spk = &LOADED_SPK.try_read().unwrap(); let jd = 2451545.0; let mut pos: Vec = Vec::new(); let mut vel: Vec = Vec::new(); diff --git a/src/kete_core/src/propagation/mod.rs b/src/kete_core/src/propagation/mod.rs index 4ca1e06..9ee4dd2 100644 --- a/src/kete_core/src/propagation/mod.rs +++ b/src/kete_core/src/propagation/mod.rs @@ -7,7 +7,7 @@ use crate::constants::{MASSES, PLANETS, SIMPLE_PLANETS}; use crate::errors::Error; use crate::frames::Frame; use crate::prelude::{Desig, KeteResult}; -use crate::spice::get_spk_singleton; +use crate::spice::LOADED_SPK; use crate::state::State; use nalgebra::{DVector, Vector3}; @@ -75,7 +75,7 @@ pub fn propagate_n_body_spk( ) -> KeteResult { let center = state.center_id; let frame = state.frame; - let spk = get_spk_singleton().try_read().unwrap(); + let spk = &LOADED_SPK.try_read().unwrap(); spk.try_change_center(&mut state, 0)?; state.try_change_frame_mut(Frame::Equatorial)?; @@ -164,7 +164,7 @@ pub fn propagate_n_body_vec( let mut desigs: Vec = Vec::new(); let planet_states = planet_states.unwrap_or_else(|| { - let spk = get_spk_singleton().try_read().unwrap(); + let spk = &LOADED_SPK.try_read().unwrap(); let mut planet_states = Vec::with_capacity(SIMPLE_PLANETS.len()); for obj in SIMPLE_PLANETS.iter() { let planet = spk diff --git a/src/kete_core/src/spice/pck.rs b/src/kete_core/src/spice/pck.rs index 0928821..cac9b87 100644 --- a/src/kete_core/src/spice/pck.rs +++ b/src/kete_core/src/spice/pck.rs @@ -10,10 +10,9 @@ use super::pck_segments::PckSegment; use crate::errors::{Error, KeteResult}; use crate::frames::Frame; use crossbeam::sync::ShardedLock; +use lazy_static::lazy_static; use std::io::Cursor; -use std::mem::MaybeUninit; -use std::sync::Once; const PRELOAD_PCK: &[&[u8]] = &[ include_bytes!("../../data/earth_000101_240215_231123.bpc"), @@ -78,28 +77,15 @@ impl PckCollection { } } -/// Get the PCK singleton. -/// This is a RwLock protected PCKCollection, and must be `.try_read().unwrapped()` for any -/// read-only cases. -pub fn get_pck_singleton() -> &'static PckSingleton { - // Create an uninitialized static - static mut SINGLETON: MaybeUninit = MaybeUninit::uninit(); - static ONCE: Once = Once::new(); - - unsafe { - ONCE.call_once(|| { - let mut files = PckCollection { - segments: Vec::new(), - }; - files.reset(); - let singleton: PckSingleton = ShardedLock::new(files); - // Store it to the static var, i.e. initialize it - #[allow(static_mut_refs)] - let _ = SINGLETON.write(singleton); - }); - - // Now we give out a shared reference to the data, which is safe to use concurrently. - #[allow(static_mut_refs)] - SINGLETON.assume_init_ref() - } +lazy_static! { + /// PCK singleton. + /// This is a RwLock protected PCKCollection, and must be `.try_read().unwrapped()` for any + /// read-only cases. + pub static ref LOADED_PCK: PckSingleton = { + let mut files = PckCollection { + segments: Vec::new(), + }; + files.reset(); + ShardedLock::new(files) + }; } diff --git a/src/kete_core/src/spice/spk.rs b/src/kete_core/src/spice/spk.rs index 59715fa..be7d720 100644 --- a/src/kete_core/src/spice/spk.rs +++ b/src/kete_core/src/spice/spk.rs @@ -1,17 +1,17 @@ //! Loading and reading of states from JPL SPK kernel files. //! //! SPKs are intended to be loaded into a singleton which is accessible via the -//! [`get_spk_singleton`] function defined below. This singleton is wrapped in a RwLock, +//! [`LOADED_SPK`] function defined below. This singleton is wrapped in a RwLock, //! meaning before its use it must by unwrapped. A vast majority of intended use cases //! will only be the read case. //! //! Here is a small worked example: //! ``` -//! use kete_core::spice::get_spk_singleton; +//! use kete_core::spice::LOADED_SPK; //! use kete_core::frames::Frame; //! //! // get a read-only reference to the [`SegmentCollection`] -//! let singleton = get_spk_singleton().try_read().unwrap(); +//! let singleton = LOADED_SPK.try_read().unwrap(); //! //! // get the state of 399 (Earth) with respect to the Sun (10) //! let state = singleton.try_get_state(399, 2451545.0, 10, Frame::Ecliptic); @@ -24,13 +24,12 @@ use crate::errors::Error; use crate::frames::Frame; use crate::prelude::KeteResult; use crate::state::State; +use lazy_static::lazy_static; use pathfinding::prelude::dijkstra; use std::collections::{HashMap, HashSet}; use crossbeam::sync::ShardedLock; use std::io::Cursor; -use std::mem::MaybeUninit; -use std::sync::Once; const PRELOAD_SPKS: &[&[u8]] = &[ include_bytes!("../../data/de440s.bsp"), @@ -304,33 +303,19 @@ impl SpkCollection { } } -/// Get the SPK singleton. -/// This is a RwLock protected SPKCollection, and must be `.try_read().unwrapped()` for any -/// read-only cases. -/// -/// This singleton starts initialized with preloaded SPK files for the planets. -pub fn get_spk_singleton() -> &'static SpkSingleton { - // Create an uninitialized static - static mut SINGLETON: MaybeUninit = MaybeUninit::uninit(); - static ONCE: Once = Once::new(); - - unsafe { - ONCE.call_once(|| { - let mut segments: SpkCollection = SpkCollection { - map_cache: HashMap::new(), - nodes: HashMap::new(), - segments: Vec::new(), - }; - segments.reset(); - let singleton: SpkSingleton = ShardedLock::new(segments); - // Store it to the static var, i.e. initialize it - #[allow(static_mut_refs)] - let _ = SINGLETON.write(singleton); - }); - - // Now we give out a shared reference to the data, which is safe to use - // concurrently. - #[allow(static_mut_refs)] - SINGLETON.assume_init_ref() - } +lazy_static! { + /// SPK singleton. + /// This is a RwLock protected SPKCollection, and must be `.try_read().unwrapped()` for any + /// read-only cases. + /// + /// This singleton starts initialized with preloaded SPK files for the planets. + pub static ref LOADED_SPK: SpkSingleton = { + let mut segments: SpkCollection = SpkCollection { + map_cache: HashMap::new(), + nodes: HashMap::new(), + segments: Vec::new(), + }; + segments.reset(); + ShardedLock::new(segments) + }; } diff --git a/src/kete_core/src/spice/spk_segments.rs b/src/kete_core/src/spice/spk_segments.rs index 1885681..aa3fc67 100644 --- a/src/kete_core/src/spice/spk_segments.rs +++ b/src/kete_core/src/spice/spk_segments.rs @@ -490,7 +490,6 @@ impl SpkSegmentType9 { /// /// #[derive(Debug)] -#[allow(unused)] pub struct SpkSegmentType10 { /// Generic Segments are a collection of a few different directories: /// `Packets` are where Type 10 stores the TLE values. @@ -505,7 +504,6 @@ pub struct SpkSegmentType10 { geopotential: Geopotential, } -#[allow(unused)] impl SpkSegmentType10 { #[inline(always)] fn get_times(&self) -> &[f64] { @@ -527,7 +525,7 @@ impl SpkSegmentType10 { .naive_utc(), ); - /// use the provided goepotential even if it is not correct. + // use the provided goepotential even if it is not correct. let orbit_0 = Orbit::from_kozai_elements( &self.geopotential, inclination,