Merge pull request #24 from NREL/doc-updates

Doc updates

rust/altrios-core/src/consist/locomotive/conventional_loco.rs

@@ -22,6 +22,8 @@ use crate::imports::*;
 )]
 /// Conventional locomotive
 pub struct ConventionalLoco {
+    // The fields in this struct are all locally defined structs and are therefore not documented in
+    // this context
     #[has_state]
     pub fc: FuelConverter,
     #[has_state]
@@ -43,6 +45,12 @@ impl ConventionalLoco {
         }
     }
 
+    /// # Arguments
+    /// - `pwr_out_req`: power required at the wheel/rail interface
+    /// - `dt`: time step size
+    /// - `engine_on`: whether engine is on (i.e. rotating and consuming at least idle fuel)
+    /// - `pwr_aux`: power demand for auxilliary systems
+    /// - `assert_limits`: whether to fail when powertrain capabilities are exceeded
     pub fn solve_energy_consumption(
         &mut self,
         pwr_out_req: si::Power,

rust/altrios-core/src/lin_search_hint.rs

@@ -1,6 +1,7 @@
 use crate::imports::*;
 use crate::si;
 
+// Per Geordie, we could probably get rid of this `DirT` code
 pub type DirT = u8;
 
 #[allow(non_snake_case)]
@@ -14,11 +15,17 @@ pub mod Dir {
     pub const Bwd: DirT = 2;
 }
 
+/// Has method that returns offset from start of PathTpc
 pub trait GetOffset {
+    /// Returns offset from start of PathTpc
     fn get_offset(&self) -> si::Length;
 }
 
+/// Contains method to calculate the index immediately before `offset` given the previous calculated
+/// index, `idx`, and a direction `DirT`.
 pub trait LinSearchHint {
+    /// Calculate the index immediately before `offset` given the previous calculated index, `idx`,
+    /// and a direction `DirT`.
     fn calc_idx<const DIR: DirT>(&self, offset: si::Length, idx: usize) -> anyhow::Result<usize>;
 }
 

rust/altrios-core/src/meet_pass.rs

@@ -1,3 +1,4 @@
+#![warn(missing_docs)]
 //! Meet-pass
 //!
 //! # Nomenclature

rust/altrios-core/src/meet_pass/disp_structs.rs

@@ -1,13 +1,16 @@
+/// TODO: Geordie, provide doc string for `disp_structs`, just a short sentence or two explaining
+/// what this module comprises
 use super::{disp_imports::*, est_times::EstTime};
 
+// TODO: Geordie, what's goin on with these TODOs?
 // TODO:  Could possibly implement this as Option<NonZeroU16>, and...
 pub type EstIdx = u32;
 // TODO:  This as None
 pub const EST_IDX_NA: EstIdx = 0;
 
 /// Type of estimated time node
 #[derive(Debug, Default, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, SerdeAPI)]
-#[repr(u8)]
+#[repr(u8)] // TODO: Geordie, could you explain what this does by replacing this comment?
 pub enum EstType {
     /// Train arrives at entry point to link
     Arrive,
@@ -58,17 +61,19 @@ pub struct DispAuth {
     pub arrive_entry: si::Time,
     /// Arrive Exit Time
     pub arrive_exit: si::Time,
-    /// Clear Entry Time
+    /// Clear Entry Time, TODO: Geordie, what does "Clear" mean here?
     pub clear_entry: si::Time,
-    /// Clear Exit Time
+    /// Clear Exit Time, TODO: Geordie, what does "Clear" mean here?
     pub clear_exit: si::Time,
 
-    /// Offset Front
+    /// Offset (distance) between start of track link and front of train
+    /// TODO: Geordie, is the above accurate?
     pub offset_front: si::Length,
-    /// Offset Back
+    /// Offset (distance) between start of track link and front of train
+    /// TODO: Geordie, is the above accurate?
     pub offset_back: si::Length,
 
-    /// Train Index
+    // this is a locally defined package so it's probably ok without a field-level doc string
     pub train_idx: TrainIdx,
 }
 
@@ -83,6 +88,7 @@ impl DispAuth {
 }
 
 impl Default for DispAuth {
+    /// TODO: Geordie, explain why `f64::INFINITY` is being used below.  
     fn default() -> Self {
         Self {
             arrive_entry: f64::INFINITY * uc::S,
@@ -99,6 +105,7 @@ impl Default for DispAuth {
 type TrainIdxsViewIdx = u32;
 
 #[derive(Debug, Default, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash, SerdeAPI)]
+/// TODO: Geordie, put a doc string here
 pub struct TrainIdxsView {
     pub idx_begin: TrainIdxsViewIdx,
     pub idx_end: TrainIdxsViewIdx,
@@ -120,6 +127,7 @@ impl TrainIdxsView {
 }
 
 #[derive(Debug, Default, Clone, Copy, Serialize, Deserialize, SerdeAPI)]
+/// TODO: Geordie, put a doc string here
 pub struct EstTimeStatus {
     pub train_idxs_view: TrainIdxsView,
     pub link_idx: LinkIdx,
@@ -153,8 +161,9 @@ impl EstTimeStatus {
     }
 }
 
-// TODO: switch to using single train_idx per node with duplicate disp_node_idx
+// TODO: Geordie, switch to using single train_idx per node with duplicate disp_node_idx
 #[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq, Eq, SerdeAPI)]
+/// TODO: Geordie, put a doc string here and adddress the todo above
 pub struct DivergeNode {
     pub train_idx: TrainIdx,        // default to: None, which is 0.
     pub disp_node_idx: DispNodeIdx, // default to: None, which is 0.
@@ -170,7 +179,10 @@ impl DivergeNode {
 }
 
 #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, SerdeAPI)]
+/// TODO: Geordie, put a doc string here.  What is `Disp` short for in this context?
 pub struct DispNode {
+    /// TODO: Geordie, what does this offset represent?   distance from beginning of link to front
+    /// of train?
     pub offset: si::Length,
     pub time_pass: si::Time, // = units::numPosInf * units::s
 

rust/altrios-core/src/meet_pass/dispatch.rs

@@ -1,3 +1,5 @@
+//! TODO: Geordie, put a doc string here
+
 use crate::combo_error::ComboErrors;
 use crate::train::LinkIdxTime;
 
@@ -7,6 +9,7 @@ use crate::train::LinkIdxTimeVec;
 
 #[readonly::make]
 #[derive(Debug, PartialEq, Clone, Copy)]
+/// TODO: Geordie, put a doc string here.  What is `Disp` in full word?  What does Next mean here?
 struct TrainDispNext {
     pub time: si::Time,
     pub train_idx: TrainIdx,
@@ -90,6 +93,11 @@ fn check_deadlock(
     }
 }
 
+
+/// TODO: Geordie, put a doc string here explaining what this does
+/// # Arguments (excluding args with custom structs)
+/// - `print_train_move`: TODO: Geordie, what does this do?
+/// - `print_train_exit`: TODO: Geordie, what does this do?
 pub fn run_dispatch(
     network: &[Link],
     speed_limit_train_sims: &[SpeedLimitTrainSim],
@@ -257,6 +265,10 @@ pub fn run_dispatch(
 }
 
 #[cfg_attr(feature = "pyo3", pyfunction(name = "run_dispatch"))]
+/// TODO: Geordie, put a doc string here explaining what this does
+/// # Arguments
+/// - `print_train_move`: TODO: Geordie, what does this do?
+/// - `print_train_exit`: TODO: Geordie, what does this do?
 pub fn run_dispatch_py(
     network: Vec<Link>,
     speed_limit_train_sims: crate::train::SpeedLimitTrainSimVec,

rust/altrios-core/src/meet_pass/est_times.rs

@@ -22,14 +22,15 @@ pub struct EstTime {
     /// Speed at which the train will pass this node assuming no delays
     pub speed: si::Velocity,
 
-    /// Index of the next node in the network when traveling along the shortest path from this node
+    // TODO: Geordie, I reworded these to be consistent with my understanding
+    /// Index of link leaving the next node in the network when traveling along the shortest path from this node
     pub idx_next: EstIdx,
-    /// Index of the alternate next node (if it exists)
+    /// Index of alternative link leaving next node (if it exists)  
     /// Used if the shortest path is blocked up ahead
     pub idx_next_alt: EstIdx,
-    /// Index of the previous node if the shortest path was taken to reach this node
+    /// Index of link leaving the previous node if the shortest path was taken to reach this node
     pub idx_prev: EstIdx,
-    /// Index of the alternate previous node (if it exists)
+    /// Index of link leaving the alternate previous node (if it exists)
     pub idx_prev_alt: EstIdx,
 
     /// Combination of link index and est type for this node
@@ -64,6 +65,7 @@ impl Default for EstTime {
         (self.val.last().unwrap().time_sched - self.val.first().unwrap().time_sched).get::<si::hour>()
     }
 )]
+// TODO: Geordie, what does the `Net` mean in this context?
 pub struct EstTimeNet {
     #[api(skip_get, skip_set)]
     pub val: Vec<EstTime>,
@@ -90,6 +92,10 @@ pub fn check_od_pair_valid(
 
 /// Get link indexes that lead to the destination (CURRENTLY ALLOWS LOOPS THAT
 /// ARE TOO SMALL TO FIT THE TRAIN!)
+/// # Arguments
+/// - `origs`: TODO: Geordie, put some words here
+/// - `dests`: TODO: Geordie, put some words here
+/// - `links`: TODO: Geordie, put some words here.  What do the links represent in this context?  
 pub fn get_link_idx_options(
     origs: &[Location],
     dests: &[Location],

rust/altrios-core/src/meet_pass/est_times/est_time_structs.rs

@@ -1,28 +1,40 @@
+//! TODO: Geordie, provide doc for `est_time_structs`, just a short description
+
 use super::super::disp_imports::*;
 
 #[readonly::make]
 #[derive(Debug, Clone, Copy, Serialize, Deserialize, SerdeAPI)]
+/// TODO: Geordie, update this doc string
 pub(super) struct EstJoinPath {
+    /// TODO: Geordie, provide doc string.  What does `_match` mean here?
     pub link_idx_match: LinkIdx,
+    /// TODO: Geordie, provide doc string.  What do `_next` and `Est` mean here?
     pub est_idx_next: EstIdx,
 }
 
 impl EstJoinPath {
+    /// # Arguments
+    /// see fields of [Self]
     pub fn new(link_idx_match: LinkIdx, est_idx_next: EstIdx) -> Self {
         Self {
             link_idx_match,
             est_idx_next,
         }
     }
+    /// TODO: Geordie, doc string!
     pub fn has_space_match(&self) -> bool {
         self.link_idx_match.is_fake()
     }
 }
 
 #[derive(Debug, Default, Clone, Copy, Serialize, Deserialize, SerdeAPI)]
+/// TODO: Geordie, doc string.  Explain what this struct is and why you created it
 pub(super) struct SimpleState {
+    /// TODO: Geordie, doc string, time for what?
     pub time: si::Time,
+    /// TODO: Geordie, doc string, offset of what?  Train front?
     pub offset: si::Length,
+    /// TODO: Geordie, doc string, speed of what?  train?
     pub speed: si::Velocity,
 }
 
@@ -37,6 +49,7 @@ impl SimpleState {
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, SerdeAPI)]
+/// TODO: Geordie, doc string.  Explain what this struct is and why you created it.  
 pub(super) struct SavedSim {
     pub train_sim: Box<SpeedLimitTrainSim>,
     pub join_paths: Vec<EstJoinPath>,
@@ -50,6 +63,7 @@ impl SavedSim {
             train_sim.state.offset < train_sim.offset_end() - uc::MI * 5.0
                 || (
                     train_sim.is_finished()
+                    // TODO: Geordie, has this comment been addressed?
                     // this needs to be reconsidered.  The issue is determining when SpeedLimitTrainSim is finished.
                         && train_sim.state.speed > si::Velocity::ZERO
                     // train_sim.state.offset
@@ -59,6 +73,7 @@ impl SavedSim {
 
         movement.clear();
         movement.push(SimpleState::from_train_state(&self.train_sim.state));
+        // TODO: Geordie, does this comment even make sense?
         // TODO: Tighten up this bound using braking points.
         while condition(&self.train_sim) {
             self.train_sim.step()?;

rust/altrios-core/src/meet_pass/est_times/update_times.rs

@@ -1,12 +1,19 @@
+//! TODO: Geordie, provide doc for `update_times`, just a short description
+
 use super::super::disp_imports::*;
 
-#[readonly::make]
+#[readonly::make] // TODO: Geordie, update this comment to explain this attribute
 #[derive(Debug, PartialEq, Clone, Copy)]
+/// TODO: Geordie, put a doc string here
 struct EstTimeNext {
+    /// TODO: Geordie, put a doc string here
     pub time_next: si::Time,
+    /// TODO: Geordie, put a doc string here
     pub est_idx: EstIdx,
 }
 impl EstTimeNext {
+    /// # Arguments
+    /// see fields of [Self]
     pub fn new(time_next: si::Time, est_idx: EstIdx) -> Self {
         assert!(!time_next.is_nan());
         EstTimeNext { time_next, est_idx }
@@ -33,12 +40,17 @@ impl Ord for EstTimeNext {
 
 #[readonly::make]
 #[derive(Debug, PartialEq, PartialOrd, Clone, Copy)]
+/// TODO: Geordie, provide doc string
 struct EstTimePrev {
+    /// TODO: Geordie, doc string, time for what?
     pub time_prev: si::Time,
+    /// TODO: Geordie, provide doc string.  What does `_sub` mean?
     pub time_sub: si::Time,
     pub est_idx: EstIdx,
 }
 impl EstTimePrev {
+    /// # Arguments
+    /// see fields of [Self]
     pub fn new(time_prev: si::Time, time_sub: si::Time, est_idx: EstIdx) -> Self {
         assert!(!time_prev.is_nan());
         assert!(!time_sub.is_nan());
@@ -59,8 +71,8 @@ impl Ord for EstTimePrev {
     }
 }
 
-/// Run shortest path forward on estimated time network
-/// This adjusts the linking of all join nodes and sets the scheduled times
+/// Run shortest path forward on estimated time network.  
+/// This adjusts the linking of all join nodes and sets the scheduled times.
 pub(super) fn update_times_forward(est_times: &mut [EstTime], time_depart: si::Time) {
     est_times[0].time_sched = time_depart;
     est_times[1].time_sched = time_depart;
@@ -144,9 +156,9 @@ pub(super) fn update_times_forward(est_times: &mut [EstTime], time_depart: si::T
     }
 }
 
-/// Run shortest path backward on estimated time network
+/// Run shortest path backward on estimated time network.  
 /// This adjusts the linking of all split nodes
-/// and adjusts the scheduled times to reflects the shortest paths
+/// and adjusts the scheduled times to reflects the shortest paths.
 pub(super) fn update_times_backward(est_times: &mut [EstTime]) {
     let mut is_est_passed = vec![false; est_times.len()];