Skip to content

Commit

Permalink
Better error logging with snafu (#115)
Browse files Browse the repository at this point in the history
# Summary

**This is a major breaking change.** Closes #32 .

## Architectural changes

- DAF no longer carries an explicit copy of FileRecord or NameRecord. These were 1024 bytes each. Now, they are parsed on demand from the borrowed underlying bytes.
- DataSet now uses Bytes instead of &[u8]. This means the data can be loaded from a static byte array or copied to the heap.
- Almanac now moves the DAF datasets (SPK and BPC) into itself. However, it will still unload them magically when going out of scope. Thanks to the DAF change above, this means that the Almanac size in memory stays about the same going from 5008 bytes to 6320 bytes, but while providing a much more ergonomic usage.
- `Context` renamed to `Almanac`. Too many libraries use the term "context" and Almanac refers to ancient astronomy, so it's relevant.
 - `FrameTrait` is removed in favor of the concrete `Frame` structure. This structure now includes a number of optional fields.
 - We're no longer trying to be no-std at the moment. It's a lot of work if we want errors to provide context to the user. As much as possible, I've used `&'static str` and add a lifetime, but there were lifetime conflicts between `'static` and the other lifetime I was using.

## New features

- `snafu` replaces `thiserror` everywhere.
- New `FrameUid` structure. It is designed to be a unique identifier of Frames. This is useful to retrieve the frame information from the loaded Almanac.
- CartesianState is also exported as an Orbit for simplicity.
- Orbital computations require planetary data to be loaded from a converted SPICE TPC file. All computations checked for validity before returning.

### Logging

`AniseError` no longer exists. Instead, we now follow the SNAFU philosophy of (almost) one error type per module. In practice, these error types are `pub(crate)` because a given error may happen in many different contexts.

#### Example

In `de440s_translation_verif_venus2emb`, change the epoch year from `2002` to `1800` and print the error, you'll now get something somewhat useful:
```
 ERROR anise::almanac::spk > Almanach: No summary 2 valid at epoch 1801-01-01T00:00:00 UTC
when searching for SPK summary caused DAF/SPK: summary 2 not present or does not cover requested epoch of 1801-01-01T00:00:00 UTC
```

## Modifications
### Frame printing
This is the previous printing, and it isn't short enough for most usages:

```
[Earth orientation 1 (μ = 398600.435436096 km3/s, eq. radius = 6378.14 km, polar radius = 6356.75 km, f = 0.0033536422844278)] 2000-01-01T13:39:27.999998123 UTC        position = [7085.562191, -411.115207, 1249.629706] km   velocity = [0.423308, 7.340402, 1.066334] km/s
```

This is from a Hermite BSP generated by GMAT. For some reason, the reference frame is `1`, and I need to figure out why it isn't `0`.
```
SPK Summary for TGT=-10000001 CTR=399 FRM=1 from 2000-01-01T12:00:32.183927295 ET to 2000-01-01T15:20:32.183931521 ET
```
  • Loading branch information
ChristopherRabotin authored Sep 22, 2023
1 parent 6724626 commit 3789ea3
Show file tree
Hide file tree
Showing 96 changed files with 3,935 additions and 3,827 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,13 @@ jobs:
- name: Rust-SPICE JPL DE validation
run: |
RUST_BACKTRACE=1 RUST_LOG=debug cargo test validate_jplde_de440s --features validation --release -- --nocapture --ignored
RUST_BACKTRACE=1 RUST_LOG=debug cargo test validate_jplde_de440_full --features validation --release -- --nocapture --ignored
RUST_BACKTRACE=1 RUST_LOG=debug cargo test validate_jplde_de440s --features spkezr_validation --release -- --nocapture --ignored
RUST_BACKTRACE=1 RUST_LOG=debug cargo test validate_jplde_de440_full --features spkezr_validation --release -- --nocapture --ignored
- name: Rust-SPICE hermite validation
run: |
RUST_BACKTRACE=1 RUST_LOG=debug cargo test validate_hermite_type13_from_gmat --features validation --release -- --nocapture --ignored
RUST_BACKTRACE=1 RUST_LOG=debug cargo test validate_hermite_type13_with_varying_segment_sizes --features validation --release -- --nocapture --ignored
RUST_BACKTRACE=1 RUST_LOG=debug cargo test validate_hermite_type13_from_gmat --features spkezr_validation --release -- --nocapture --ignored
RUST_BACKTRACE=1 RUST_LOG=debug cargo test validate_hermite_type13_with_varying_segment_sizes --features spkezr_validation --release -- --nocapture --ignored
# Now analyze the results and create pretty plots
- uses: actions/setup-python@v4
Expand Down
18 changes: 10 additions & 8 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ description = "ANISE provides a toolkit and files for Attitude, Navigation, Inst
homepage = "https://github.com/anise-toolkit/"
documentation = "https://docs.rs/anise/"
repository = "https://github.com/anise-toolkit/anise.rs"
keywords = ["attitude","navigation","instrument", "spacecraft", "ephemeris"]
keywords = ["attitude", "navigation", "instrument", "spacecraft", "ephemeris"]
categories = ["science", "simulation"]
readme = "README.md"
license = "MPL-2.0"
Expand All @@ -19,33 +19,35 @@ exclude = ["cspice"]
hifitime = "3.8"
memmap2 = "0.7.0"
crc32fast = "1.3.0"
der = {version = "0.7.8", features = ["derive", "alloc", "real"]}
clap = {version = "3.1", features = ["derive"]}
der = { version = "0.7.8", features = ["derive", "alloc", "real"] }
clap = { version = "3.1", features = ["derive"] }
thiserror = "1.0"
log = "0.4"
pretty_env_logger = "0.5"
tabled = "0.14"
const_format = "0.2"
nalgebra = "0.32"
approx = "0.5.1"
zerocopy = {version = "0.7.3", features = ["derive"]}
zerocopy = { version = "0.7.3", features = ["derive"] }
bytes = "1.4.0"
snafu = "0.7.4"
snafu = { version = "0.7.4", features = ["backtrace"] }
lexical-core = "0.8.5"
heapless = "0.7.16"
rstest = "0.18.2"

[dev-dependencies]
rust-spice = "0.7.4"
parquet = "46.0.0"
arrow = "46.0.0"
criterion = "0.5"
iai = "0.1"
polars = {version = "0.33", features = ["lazy", "parquet"]}
polars = { version = "0.33", features = ["lazy", "parquet"] }
rayon = "1.7"

[features]
default = ["validation"]
validation = [] # Enabling this flag significantly reduces compilation times due to Arrow and Polars.
default = ["spkezr_validation"]
# Enabling this flag significantly reduces compilation times due to Arrow and Polars.
spkezr_validation = []

[profile.bench]
debug = true
Expand Down
6 changes: 2 additions & 4 deletions benches/crit_jpl_ephemerides.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ use anise::{
};
use criterion::{black_box, criterion_group, criterion_main, Criterion};

use spice;

const NUM_QUERIES_PER_PAIR: f64 = 100.0;

fn benchmark_spice_single_hop_type2_cheby(time_it: TimeSeries) {
Expand All @@ -24,7 +22,7 @@ fn benchmark_spice_single_hop_type2_cheby(time_it: TimeSeries) {
fn benchmark_anise_single_hop_type2_cheby(ctx: &Almanac, time_it: TimeSeries) {
for epoch in time_it {
black_box(
ctx.translate_from_to_km_s_geometric(EARTH_J2000, LUNA_J2000, epoch)
ctx.translate_from_to_geometric(EARTH_J2000, LUNA_J2000, epoch)
.unwrap(),
);
}
Expand All @@ -40,7 +38,7 @@ pub fn criterion_benchmark(c: &mut Criterion) {
let path = "./data/de440s.bsp";
let buf = file2heap!(path).unwrap();
let spk = SPK::parse(buf).unwrap();
let ctx = Almanac::from_spk(&spk).unwrap();
let ctx = Almanac::from_spk(spk).unwrap();

// Load SPICE data
spice::furnsh("data/de440s.bsp");
Expand Down
8 changes: 3 additions & 5 deletions benches/crit_spacecraft_ephemeris.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ use anise::{constants::frames::EARTH_J2000, file2heap, prelude::*};

use criterion::{black_box, criterion_group, criterion_main, Criterion};

use spice;

const NUM_QUERIES: f64 = 100.0;

fn benchmark_spice_single_hop_type13_hermite(time_it: TimeSeries) {
Expand All @@ -27,7 +25,7 @@ fn benchmark_anise_single_hop_type13_hermite(ctx: &Almanac, time_it: TimeSeries)
let my_sc_j2k = Frame::from_ephem_j2000(-10000001);
for epoch in time_it {
black_box(
ctx.translate_from_to_km_s_geometric(my_sc_j2k, EARTH_J2000, epoch)
ctx.translate_from_to_geometric(my_sc_j2k, EARTH_J2000, epoch)
.unwrap(),
);
}
Expand All @@ -46,9 +44,9 @@ pub fn criterion_benchmark(c: &mut Criterion) {
let buf = file2heap!("data/gmat-hermite.bsp").unwrap();
let spacecraft = SPK::parse(buf).unwrap();

let ctx = Almanac::from_spk(&spk)
let ctx = Almanac::from_spk(spk)
.unwrap()
.load_spk(&spacecraft)
.load_spk(spacecraft)
.unwrap();

// Load SPICE data
Expand Down
5 changes: 2 additions & 3 deletions benches/iai_jpl_ephemerides.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use anise::{
};

use iai::black_box;
use spice;

const NUM_QUERIES_PER_PAIR: f64 = 100.0;

Expand Down Expand Up @@ -40,11 +39,11 @@ fn benchmark_anise_single_hop_type2_cheby() {
let path = "./data/de440s.bsp";
let buf = file2heap!(path).unwrap();
let spk = SPK::parse(buf).unwrap();
let ctx = Almanac::from_spk(&spk).unwrap();
let ctx = Almanac::from_spk(spk).unwrap();

for epoch in time_it {
black_box(
ctx.translate_from_to_km_s_geometric(EARTH_J2000, LUNA_J2000, epoch)
ctx.translate_from_to_geometric(EARTH_J2000, LUNA_J2000, epoch)
.unwrap(),
);
}
Expand Down
7 changes: 3 additions & 4 deletions benches/iai_spacecraft_ephemeris.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use anise::{constants::frames::EARTH_J2000, file2heap, prelude::*};

use iai::black_box;
use spice;

fn benchmark_spice_single_hop_type13_hermite() {
let epoch = Epoch::from_gregorian_hms(2000, 1, 1, 14, 0, 0, TimeScale::UTC);
Expand Down Expand Up @@ -30,15 +29,15 @@ fn benchmark_anise_single_hop_type13_hermite() {
let buf = file2heap!("data/gmat-hermite.bsp").unwrap();
let spacecraft = SPK::parse(buf).unwrap();

let ctx = Almanac::from_spk(&spk)
let ctx = Almanac::from_spk(spk)
.unwrap()
.load_spk(&spacecraft)
.load_spk(spacecraft)
.unwrap();

let my_sc_j2k = Frame::from_ephem_j2000(-10000001);

black_box(
ctx.translate_from_to_km_s_geometric(my_sc_j2k, EARTH_J2000, epoch)
ctx.translate_from_to_geometric(my_sc_j2k, EARTH_J2000, epoch)
.unwrap(),
);
}
Expand Down
104 changes: 81 additions & 23 deletions src/almanac/bpc.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* ANISE Toolkit
* Copyright (C) 2021-2022 Christopher Rabotin <[email protected]> et al. (cf. AUTHORS.md)
* Copyright (C) 2021-2023 Christopher Rabotin <[email protected]> et al. (cf. AUTHORS.md)
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
Expand All @@ -10,16 +10,17 @@

use hifitime::Epoch;

use crate::errors::AniseError;
use crate::naif::daf::DAFError;
use crate::naif::pck::BPCSummaryRecord;
use crate::naif::BPC;
use crate::orientations::OrientationError;
use log::error;

use super::{Almanac, MAX_LOADED_BPCS};

impl<'a: 'b, 'b> Almanac<'a> {
/// Loads a Binary Planetary Constants kernel.
pub fn load_bpc(&self, bpc: &'b BPC) -> Result<Almanac<'b>, AniseError> {
pub fn load_bpc(&self, bpc: BPC) -> Result<Almanac<'b>, OrientationError> {
// This is just a bunch of pointers so it doesn't use much memory.
let mut me = self.clone();
let mut data_idx = MAX_LOADED_BPCS;
Expand All @@ -30,15 +31,17 @@ impl<'a: 'b, 'b> Almanac<'a> {
}
}
if data_idx == MAX_LOADED_BPCS {
return Err(AniseError::StructureIsFull);
return Err(OrientationError::StructureIsFull {
max_slots: MAX_LOADED_BPCS,
});
}
me.bpc_data[data_idx] = Some(bpc);
Ok(me)
}

pub fn num_loaded_bpc(&self) -> usize {
let mut count = 0;
for maybe in self.bpc_data {
for maybe in &self.bpc_data {
if maybe.is_none() {
break;
} else {
Expand All @@ -54,93 +57,148 @@ impl<'a: 'b, 'b> Almanac<'a> {
&self,
name: &str,
epoch: Epoch,
) -> Result<(&BPCSummaryRecord, usize, usize), AniseError> {
) -> Result<(&BPCSummaryRecord, usize, usize), OrientationError> {
for (no, maybe_bpc) in self
.bpc_data
.iter()
.take(self.num_loaded_bpc())
.rev()
.enumerate()
{
let bpc = maybe_bpc.unwrap();
let bpc = maybe_bpc.as_ref().unwrap();
if let Ok((summary, idx_in_bpc)) = bpc.summary_from_name_at_epoch(name, epoch) {
return Ok((summary, no, idx_in_bpc));
}
}

// If we're reached this point, there is no relevant summary at this epoch.
error!("Context: No summary {name} valid at epoch {epoch}");
Err(AniseError::MissingInterpolationData(epoch))
error!("Almanac: No summary {name} valid at epoch {epoch}");
Err(OrientationError::BPC {
action: "searching for BPC summary",
source: DAFError::SummaryNameAtEpochError {
kind: "BPC",
name: name.to_string(),
epoch,
},
})
}

/// Returns the summary given the name of the summary record if that summary has data defined at the requested epoch
pub fn bpc_summary_at_epoch(
&self,
id: i32,
epoch: Epoch,
) -> Result<(&BPCSummaryRecord, usize, usize), AniseError> {
// TODO: Consider a return type here
) -> Result<(&BPCSummaryRecord, usize, usize), OrientationError> {
for (no, maybe_bpc) in self
.bpc_data
.iter()
.take(self.num_loaded_bpc())
.rev()
.enumerate()
{
let bpc = maybe_bpc.unwrap();
let bpc = maybe_bpc.as_ref().unwrap();
if let Ok((summary, idx_in_bpc)) = bpc.summary_from_id_at_epoch(id, epoch) {
// NOTE: We're iterating backward, so the correct BPC number is "total loaded" minus "current iteration".
return Ok((summary, self.num_loaded_bpc() - no - 1, idx_in_bpc));
}
}

error!("Context: No summary {id} valid at epoch {epoch}");
error!("Almanac: No summary {id} valid at epoch {epoch}");
// If we're reached this point, there is no relevant summary at this epoch.
Err(AniseError::MissingInterpolationData(epoch))
Err(OrientationError::BPC {
action: "searching for BPC summary",
source: DAFError::SummaryIdAtEpochError {
kind: "BPC",
id,
epoch,
},
})
}

/// Returns the summary given the name of the summary record.
pub fn bpc_summary_from_name(
&self,
name: &str,
) -> Result<(&BPCSummaryRecord, usize, usize), AniseError> {
) -> Result<(&BPCSummaryRecord, usize, usize), OrientationError> {
for (bpc_no, maybe_bpc) in self
.bpc_data
.iter()
.take(self.num_loaded_bpc())
.rev()
.enumerate()
{
let bpc = maybe_bpc.unwrap();
let bpc = maybe_bpc.as_ref().unwrap();
if let Ok((summary, idx_in_bpc)) = bpc.summary_from_name(name) {
return Ok((summary, bpc_no, idx_in_bpc));
}
}

// If we're reached this point, there is no relevant summary at this epoch.
error!("Context: No summary {name} valid");
Err(AniseError::NoInterpolationData)
error!("Almanac: No summary {name} valid");
Err(OrientationError::BPC {
action: "searching for BPC summary",
source: DAFError::SummaryNameError {
kind: "BPC",
name: name.to_string(),
},
})
}

/// Returns the summary given the name of the summary record if that summary has data defined at the requested epoch
pub fn bpc_summary(&self, id: i32) -> Result<(&BPCSummaryRecord, usize, usize), AniseError> {
// TODO: Consider a return type here
pub fn bpc_summary(
&self,
id: i32,
) -> Result<(&BPCSummaryRecord, usize, usize), OrientationError> {
for (no, maybe_bpc) in self
.bpc_data
.iter()
.take(self.num_loaded_bpc())
.rev()
.enumerate()
{
let bpc = maybe_bpc.unwrap();
let bpc = maybe_bpc.as_ref().unwrap();
if let Ok((summary, idx_in_bpc)) = bpc.summary_from_id(id) {
// NOTE: We're iterating backward, so the correct BPC number is "total loaded" minus "current iteration".
return Ok((summary, self.num_loaded_bpc() - no - 1, idx_in_bpc));
}
}

error!("Context: No summary {id} valid");
error!("Almanac: No summary {id} valid");
// If we're reached this point, there is no relevant summary
Err(AniseError::NoInterpolationData)
Err(OrientationError::BPC {
action: "searching for BPC summary",
source: DAFError::SummaryIdError { kind: "BPC", id },
})
}
}

#[cfg(test)]
mod ut_almanac_bpc {
use crate::prelude::{Almanac, Epoch};

#[test]
fn summaries_nothing_loaded() {
let almanac = Almanac::default();

let e = Epoch::now().unwrap();

assert!(
almanac.bpc_summary(0).is_err(),
"empty Almanac should report an error"
);
assert!(
almanac.bpc_summary_at_epoch(0, e).is_err(),
"empty Almanac should report an error"
);
assert!(
almanac.bpc_summary_from_name("invalid name").is_err(),
"empty Almanac should report an error"
);
assert!(
almanac
.bpc_summary_from_name_at_epoch("invalid name", e)
.is_err(),
"empty Almanac should report an error"
);
}
}
Loading

0 comments on commit 3789ea3

Please sign in to comment.