From abc25bb3632f583c344af548ed849b7b400209f9 Mon Sep 17 00:00:00 2001
From: Eric Rescorla <cym4@cdc.gov>
Date: Mon, 4 Nov 2024 16:27:39 -0800
Subject: [PATCH 1/4] Change |set_global_property_value()| to return
 Result<IxaError>. Fixes #71.

This keeps the semantics that global property values are
immutable but returns an error when you try to change one.
We might still want to change this semantic, but at least
this prevents silent failure.
---
 examples/births-deaths/parameters_loader.rs   |  4 +-
 .../parameter-loading/infection_manager.rs    |  4 +-
 .../parameter-loading/parameters_loader.rs    |  2 +-
 .../parameter-loading/transmission_manager.rs |  4 +-
 .../exposure_manager.rs                       |  6 +-
 .../infection_manager.rs                      | 10 +++-
 .../parameters_loader.rs                      |  2 +-
 .../population_loader.rs                      | 43 ++++++++++++++
 src/error.rs                                  |  4 +-
 src/global_properties.rs                      | 59 +++++++++++++------
 10 files changed, 107 insertions(+), 31 deletions(-)

diff --git a/examples/births-deaths/parameters_loader.rs b/examples/births-deaths/parameters_loader.rs
index 2dbee8f2..0d9c68be 100644
--- a/examples/births-deaths/parameters_loader.rs
+++ b/examples/births-deaths/parameters_loader.rs
@@ -32,7 +32,7 @@ define_global_property!(Foi, HashMap<AgeGroupRisk, f64>);
 
 pub fn init_parameters(context: &mut Context, file_path: &Path) -> Result<(), IxaError> {
     let parameters_json = context.load_parameters_from_json::<ParametersValues>(file_path)?;
-    context.set_global_property_value(Parameters, parameters_json.clone());
+    context.set_global_property_value(Parameters, parameters_json.clone()).unwrap();
 
     let foi_map = parameters_json
         .foi_groups
@@ -41,7 +41,7 @@ pub fn init_parameters(context: &mut Context, file_path: &Path) -> Result<(), Ix
         .map(|x| (x.group_name, x.foi))
         .collect::<HashMap<AgeGroupRisk, f64>>();
 
-    context.set_global_property_value(Foi, foi_map.clone());
+    context.set_global_property_value(Foi, foi_map.clone()).unwrap();
 
     Ok(())
 }
diff --git a/examples/parameter-loading/infection_manager.rs b/examples/parameter-loading/infection_manager.rs
index 1cc2f7ed..0a70600f 100644
--- a/examples/parameter-loading/infection_manager.rs
+++ b/examples/parameter-loading/infection_manager.rs
@@ -66,7 +66,9 @@ mod test {
         };
         let mut context = Context::new();
 
-        context.set_global_property_value(Parameters, p_values.clone());
+        context
+            .set_global_property_value(Parameters, p_values.clone())
+            .unwrap();
         context.init_random(42);
         init(&mut context);
 
diff --git a/examples/parameter-loading/parameters_loader.rs b/examples/parameter-loading/parameters_loader.rs
index aeee1127..551d2778 100644
--- a/examples/parameter-loading/parameters_loader.rs
+++ b/examples/parameter-loading/parameters_loader.rs
@@ -21,6 +21,6 @@ define_global_property!(Parameters, ParametersValues);
 
 pub fn init_parameters(context: &mut Context, file_path: &Path) -> Result<(), IxaError> {
     let parameters_json = context.load_parameters_from_json::<ParametersValues>(file_path)?;
-    context.set_global_property_value(Parameters, parameters_json);
+    context.set_global_property_value(Parameters, parameters_json)?;
     Ok(())
 }
diff --git a/examples/parameter-loading/transmission_manager.rs b/examples/parameter-loading/transmission_manager.rs
index c5ff3c9e..fc994ca7 100644
--- a/examples/parameter-loading/transmission_manager.rs
+++ b/examples/parameter-loading/transmission_manager.rs
@@ -70,7 +70,9 @@ mod test {
         };
 
         let mut context = Context::new();
-        context.set_global_property_value(Parameters, p_values);
+        context
+            .set_global_property_value(Parameters, p_values)
+            .unwrap();
         context.init_random(123);
         let pid = context.add_person(()).unwrap();
         attempt_infection(&mut context);
diff --git a/examples/time-varying-infection/exposure_manager.rs b/examples/time-varying-infection/exposure_manager.rs
index 6ac786b5..dd3785ec 100644
--- a/examples/time-varying-infection/exposure_manager.rs
+++ b/examples/time-varying-infection/exposure_manager.rs
@@ -94,7 +94,9 @@ mod test {
             output_file: ".".to_string(),
         };
         let mut context = Context::new();
-        context.set_global_property_value(Parameters, p_values);
+        context
+            .set_global_property_value(Parameters, p_values)
+            .unwrap();
         let parameters = context
             .get_global_property_value(Parameters)
             .unwrap()
@@ -132,7 +134,7 @@ mod test {
             output_file: ".".to_string(),
         };
         let mut context = Context::new();
-        context.set_global_property_value(Parameters, p_values);
+        context.set_global_property_value(Parameters, p_values).unwrap();
         let parameters = context
             .get_global_property_value(Parameters)
             .unwrap()
diff --git a/examples/time-varying-infection/infection_manager.rs b/examples/time-varying-infection/infection_manager.rs
index e87fcd2b..b6bb673e 100644
--- a/examples/time-varying-infection/infection_manager.rs
+++ b/examples/time-varying-infection/infection_manager.rs
@@ -120,7 +120,7 @@ mod test {
         };
         let mut context = Context::new();
 
-        context.set_global_property_value(Parameters, p_values);
+        context.set_global_property_value(Parameters, p_values).unwrap();
         let parameters = context
             .get_global_property_value(Parameters)
             .unwrap()
@@ -161,7 +161,9 @@ mod test {
             output_dir: ".".to_string(),
             output_file: ".".to_string(),
         };
-        context.set_global_property_value(Parameters, parameters.clone());
+        context
+            .set_global_property_value(Parameters, parameters.clone())
+            .unwrap();
         context.init_random(parameters.seed);
         for _ in 0..parameters.population {
             let person_id = context.add_person(()).unwrap();
@@ -211,7 +213,9 @@ mod test {
         let mut sum = 0.0;
         for seed in 0..n_iter {
             let mut context = Context::new();
-            context.set_global_property_value(Parameters, parameters.clone());
+            context
+                .set_global_property_value(Parameters, parameters.clone())
+                .unwrap();
             context.init_random(seed);
             init(&mut context);
             let person_id = context
diff --git a/examples/time-varying-infection/parameters_loader.rs b/examples/time-varying-infection/parameters_loader.rs
index d6871df1..b12f58dd 100644
--- a/examples/time-varying-infection/parameters_loader.rs
+++ b/examples/time-varying-infection/parameters_loader.rs
@@ -23,6 +23,6 @@ define_global_property!(Parameters, ParametersValues);
 
 pub fn init_parameters(context: &mut Context, file_path: &Path) -> Result<(), IxaError> {
     let parameters_json = context.load_parameters_from_json::<ParametersValues>(file_path)?;
-    context.set_global_property_value(Parameters, parameters_json);
+    context.set_global_property_value(Parameters, parameters_json)?;
     Ok(())
 }
diff --git a/examples/time-varying-infection/population_loader.rs b/examples/time-varying-infection/population_loader.rs
index 89f1736e..f87bca9b 100644
--- a/examples/time-varying-infection/population_loader.rs
+++ b/examples/time-varying-infection/population_loader.rs
@@ -27,3 +27,46 @@ pub fn init(context: &mut Context) {
         context.add_person(()).unwrap();
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use ixa::context::Context;
+    use ixa::global_properties::ContextGlobalPropertiesExt;
+    use ixa::people::ContextPeopleExt;
+    use ixa::random::ContextRandomExt;
+
+    use crate::parameters_loader::ParametersValues;
+
+    #[test]
+    #[should_panic(expected = "Property not initialized")]
+    fn test_person_creation_default_properties() {
+        let p_values = ParametersValues {
+            population: 1,
+            max_time: 10.0,
+            seed: 42,
+            foi: 0.15,
+            foi_sin_shift: 3.0,
+            infection_duration: 5.0,
+            report_period: 1.0,
+            output_dir: ".".to_string(),
+            output_file: ".".to_string(),
+        };
+        let mut context = Context::new();
+        context
+            .set_global_property_value(Parameters, p_values)
+            .unwrap();
+        let parameters = context.get_global_property_value(Parameters).clone();
+        context.init_random(parameters.seed);
+        init(&mut context);
+
+        let population_size = context.get_current_population();
+        assert_eq!(population_size, parameters.population);
+        for i in 0..population_size {
+            let status = context.get_person_property(context.get_person_id(i), DiseaseStatusType);
+            assert_eq!(status, DiseaseStatus::S);
+            context.get_person_property(context.get_person_id(i), InfectionTime);
+        }
+    }
+}
diff --git a/src/error.rs b/src/error.rs
index e300721a..06be26e2 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -4,8 +4,8 @@ use std::io;
 
 #[derive(Debug)]
 #[allow(clippy::module_name_repetitions)]
-/// A wrapper around other error types, as well
-/// as our specific error type.
+/// Provides `IxaError` and maps to other errors to
+/// convert to an `IxaError`
 pub enum IxaError {
     IoError(io::Error),
     JsonError(serde_json::Error),
diff --git a/src/global_properties.rs b/src/global_properties.rs
index e44652ff..72266ef1 100644
--- a/src/global_properties.rs
+++ b/src/global_properties.rs
@@ -3,7 +3,7 @@ use crate::error::IxaError;
 use serde::de::DeserializeOwned;
 use std::any::{Any, TypeId};
 use std::cell::RefCell;
-use std::collections::HashMap;
+use std::collections::{hash_map::Entry, HashMap};
 use std::fmt::Debug;
 use std::fs;
 use std::io::BufReader;
@@ -40,7 +40,7 @@ where
                 if context.get_global_property_value(T::new()).is_some() {
                     return Err(IxaError::IxaError(format!("Duplicate property {name}")));
                 }
-                context.set_global_property_value(T::new(), val);
+                let _ = context.set_global_property_value(T::new(), val);
                 Ok(())
             },
         ),
@@ -122,11 +122,14 @@ crate::context::define_data_plugin!(
 
 pub trait ContextGlobalPropertiesExt {
     /// Set the value of a global property of type T
+    ///
+    /// # Errors
+    /// Will return an error if an attempt is made to change a value.
     fn set_global_property_value<T: GlobalProperty + 'static>(
         &mut self,
         property: T,
         value: T::Value,
-    );
+    ) -> Result<(), IxaError>;
 
     /// Return value of global property T
     fn get_global_property_value<T: GlobalProperty + 'static>(
@@ -171,11 +174,14 @@ impl GlobalPropertiesDataContainer {
         &mut self,
         _property: &T,
         value: T::Value,
-    ) {
-        let _data_container = self
-            .global_property_container
-            .entry(TypeId::of::<T>())
-            .or_insert_with(|| Box::new(value));
+    ) -> Result<(), IxaError> {
+        match self.global_property_container.entry(TypeId::of::<T>()) {
+            Entry::Vacant(entry) => {
+                entry.insert(Box::new(value));
+                Ok(())
+            }
+            Entry::Occupied(_) => Err(IxaError::from("Entry already exists")),
+        }
     }
 
     #[must_use]
@@ -194,9 +200,9 @@ impl ContextGlobalPropertiesExt for Context {
         &mut self,
         property: T,
         value: T::Value,
-    ) {
+    ) -> Result<(), IxaError> {
         let data_container = self.get_data_container_mut(GlobalPropertiesPlugin);
-        data_container.set_global_property_value(&property, value);
+        data_container.set_global_property_value(&property, value)
     }
 
     #[allow(unused_variables)]
@@ -252,20 +258,35 @@ mod test {
     }
 
     define_global_property!(DiseaseParams, ParamType);
-    //Since global properties aren't mutable right now, only
-    // check that they are properly set
+
     #[test]
     fn set_get_global_property() {
         let params: ParamType = ParamType {
             days: 10,
             diseases: 2,
         };
+        let params2: ParamType = ParamType {
+            days: 11,
+            diseases: 3,
+        };
+
         let mut context = Context::new();
-        context.set_global_property_value(DiseaseParams, params.clone());
-        let global_params = context
-            .get_global_property_value(DiseaseParams)
-            .unwrap()
-            .clone();
+
+        // Set and check the stored value.
+        context
+            .set_global_property_value(DiseaseParams, params.clone())
+            .unwrap();
+        let global_params = context.get_global_property_value(DiseaseParams).unwrap().clone();
+        assert_eq!(global_params.days, params.days);
+        assert_eq!(global_params.diseases, params.diseases);
+
+        // Setting again should fail because global properties are immutable.
+        assert!(context
+            .set_global_property_value(DiseaseParams, params2.clone())
+            .is_err());
+
+        // Check that the value is unchanged.
+        let global_params = context.get_global_property_value(DiseaseParams).unwrap().clone();
         assert_eq!(global_params.days, params.days);
         assert_eq!(global_params.diseases, params.diseases);
     }
@@ -298,7 +319,9 @@ mod test {
             .load_parameters_from_json::<ParamType>(&file_path)
             .unwrap();
 
-        context.set_global_property_value(Parameters, params_json);
+        context
+            .set_global_property_value(Parameters, params_json)
+            .unwrap();
 
         let params_read = context
             .get_global_property_value(Parameters)

From c076eaa4ae7400ef55bce7c29064c1a3b92dcaab Mon Sep 17 00:00:00 2001
From: Eric Rescorla <cym4@cdc.gov>
Date: Sun, 1 Dec 2024 16:36:36 -0800
Subject: [PATCH 2/4] FMT

---
 examples/births-deaths/parameters_loader.rs          |  8 ++++++--
 examples/time-varying-infection/exposure_manager.rs  |  4 +++-
 examples/time-varying-infection/infection_manager.rs |  4 +++-
 src/global_properties.rs                             | 10 ++++++++--
 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/examples/births-deaths/parameters_loader.rs b/examples/births-deaths/parameters_loader.rs
index 0d9c68be..322fb5f9 100644
--- a/examples/births-deaths/parameters_loader.rs
+++ b/examples/births-deaths/parameters_loader.rs
@@ -32,7 +32,9 @@ define_global_property!(Foi, HashMap<AgeGroupRisk, f64>);
 
 pub fn init_parameters(context: &mut Context, file_path: &Path) -> Result<(), IxaError> {
     let parameters_json = context.load_parameters_from_json::<ParametersValues>(file_path)?;
-    context.set_global_property_value(Parameters, parameters_json.clone()).unwrap();
+    context
+        .set_global_property_value(Parameters, parameters_json.clone())
+        .unwrap();
 
     let foi_map = parameters_json
         .foi_groups
@@ -41,7 +43,9 @@ pub fn init_parameters(context: &mut Context, file_path: &Path) -> Result<(), Ix
         .map(|x| (x.group_name, x.foi))
         .collect::<HashMap<AgeGroupRisk, f64>>();
 
-    context.set_global_property_value(Foi, foi_map.clone()).unwrap();
+    context
+        .set_global_property_value(Foi, foi_map.clone())
+        .unwrap();
 
     Ok(())
 }
diff --git a/examples/time-varying-infection/exposure_manager.rs b/examples/time-varying-infection/exposure_manager.rs
index dd3785ec..478bdb5e 100644
--- a/examples/time-varying-infection/exposure_manager.rs
+++ b/examples/time-varying-infection/exposure_manager.rs
@@ -134,7 +134,9 @@ mod test {
             output_file: ".".to_string(),
         };
         let mut context = Context::new();
-        context.set_global_property_value(Parameters, p_values).unwrap();
+        context
+            .set_global_property_value(Parameters, p_values)
+            .unwrap();
         let parameters = context
             .get_global_property_value(Parameters)
             .unwrap()
diff --git a/examples/time-varying-infection/infection_manager.rs b/examples/time-varying-infection/infection_manager.rs
index b6bb673e..74b37ee9 100644
--- a/examples/time-varying-infection/infection_manager.rs
+++ b/examples/time-varying-infection/infection_manager.rs
@@ -120,7 +120,9 @@ mod test {
         };
         let mut context = Context::new();
 
-        context.set_global_property_value(Parameters, p_values).unwrap();
+        context
+            .set_global_property_value(Parameters, p_values)
+            .unwrap();
         let parameters = context
             .get_global_property_value(Parameters)
             .unwrap()
diff --git a/src/global_properties.rs b/src/global_properties.rs
index 72266ef1..dfd710d1 100644
--- a/src/global_properties.rs
+++ b/src/global_properties.rs
@@ -276,7 +276,10 @@ mod test {
         context
             .set_global_property_value(DiseaseParams, params.clone())
             .unwrap();
-        let global_params = context.get_global_property_value(DiseaseParams).unwrap().clone();
+        let global_params = context
+            .get_global_property_value(DiseaseParams)
+            .unwrap()
+            .clone();
         assert_eq!(global_params.days, params.days);
         assert_eq!(global_params.diseases, params.diseases);
 
@@ -286,7 +289,10 @@ mod test {
             .is_err());
 
         // Check that the value is unchanged.
-        let global_params = context.get_global_property_value(DiseaseParams).unwrap().clone();
+        let global_params = context
+            .get_global_property_value(DiseaseParams)
+            .unwrap()
+            .clone();
         assert_eq!(global_params.days, params.days);
         assert_eq!(global_params.diseases, params.diseases);
     }

From 0f746cfe38e685afffb9d5ffd952239fccd1d6e1 Mon Sep 17 00:00:00 2001
From: Eric Rescorla <cym4@cdc.gov>
Date: Sun, 1 Dec 2024 16:44:07 -0800
Subject: [PATCH 3/4] Fix rebase issues

---
 examples/births-deaths/infection_manager.rs   |  2 +-
 examples/births-deaths/population_manager.rs  |  4 +-
 .../population_loader.rs                      | 43 -------------------
 3 files changed, 3 insertions(+), 46 deletions(-)

diff --git a/examples/births-deaths/infection_manager.rs b/examples/births-deaths/infection_manager.rs
index c0a49d8a..c2b53b62 100644
--- a/examples/births-deaths/infection_manager.rs
+++ b/examples/births-deaths/infection_manager.rs
@@ -127,7 +127,7 @@ mod test {
 
         let mut context = Context::new();
 
-        context.set_global_property_value(Parameters, p_values.clone());
+        context.set_global_property_value(Parameters, p_values.clone()).unwrap();
         context.init_random(p_values.seed);
         init(&mut context);
 
diff --git a/examples/births-deaths/population_manager.rs b/examples/births-deaths/population_manager.rs
index 16a562aa..9e5ff4d2 100644
--- a/examples/births-deaths/population_manager.rs
+++ b/examples/births-deaths/population_manager.rs
@@ -278,7 +278,7 @@ mod test {
         };
 
         let mut context = Context::new();
-        context.set_global_property_value(Parameters, p_values.clone());
+        context.set_global_property_value(Parameters, p_values.clone()).unwrap();
         context.init_random(p_values.seed);
 
         schedule_birth(&mut context);
@@ -300,7 +300,7 @@ mod test {
         };
 
         let mut context = Context::new();
-        context.set_global_property_value(Parameters, p_values.clone());
+        context.set_global_property_value(Parameters, p_values.clone()).unwrap();
         context.init_random(p_values.seed);
         let _person = context.create_new_person(0);
         schedule_death(&mut context);
diff --git a/examples/time-varying-infection/population_loader.rs b/examples/time-varying-infection/population_loader.rs
index f87bca9b..89f1736e 100644
--- a/examples/time-varying-infection/population_loader.rs
+++ b/examples/time-varying-infection/population_loader.rs
@@ -27,46 +27,3 @@ pub fn init(context: &mut Context) {
         context.add_person(()).unwrap();
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    use ixa::context::Context;
-    use ixa::global_properties::ContextGlobalPropertiesExt;
-    use ixa::people::ContextPeopleExt;
-    use ixa::random::ContextRandomExt;
-
-    use crate::parameters_loader::ParametersValues;
-
-    #[test]
-    #[should_panic(expected = "Property not initialized")]
-    fn test_person_creation_default_properties() {
-        let p_values = ParametersValues {
-            population: 1,
-            max_time: 10.0,
-            seed: 42,
-            foi: 0.15,
-            foi_sin_shift: 3.0,
-            infection_duration: 5.0,
-            report_period: 1.0,
-            output_dir: ".".to_string(),
-            output_file: ".".to_string(),
-        };
-        let mut context = Context::new();
-        context
-            .set_global_property_value(Parameters, p_values)
-            .unwrap();
-        let parameters = context.get_global_property_value(Parameters).clone();
-        context.init_random(parameters.seed);
-        init(&mut context);
-
-        let population_size = context.get_current_population();
-        assert_eq!(population_size, parameters.population);
-        for i in 0..population_size {
-            let status = context.get_person_property(context.get_person_id(i), DiseaseStatusType);
-            assert_eq!(status, DiseaseStatus::S);
-            context.get_person_property(context.get_person_id(i), InfectionTime);
-        }
-    }
-}

From 8a081cb9044158a2fe3af13d2217558a5c4d34e0 Mon Sep 17 00:00:00 2001
From: Eric Rescorla <cym4@cdc.gov>
Date: Sun, 1 Dec 2024 16:48:05 -0800
Subject: [PATCH 4/4] FMT

---
 examples/births-deaths/infection_manager.rs  | 4 +++-
 examples/births-deaths/population_manager.rs | 8 ++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/examples/births-deaths/infection_manager.rs b/examples/births-deaths/infection_manager.rs
index c2b53b62..4657fe82 100644
--- a/examples/births-deaths/infection_manager.rs
+++ b/examples/births-deaths/infection_manager.rs
@@ -127,7 +127,9 @@ mod test {
 
         let mut context = Context::new();
 
-        context.set_global_property_value(Parameters, p_values.clone()).unwrap();
+        context
+            .set_global_property_value(Parameters, p_values.clone())
+            .unwrap();
         context.init_random(p_values.seed);
         init(&mut context);
 
diff --git a/examples/births-deaths/population_manager.rs b/examples/births-deaths/population_manager.rs
index 9e5ff4d2..6ffbb832 100644
--- a/examples/births-deaths/population_manager.rs
+++ b/examples/births-deaths/population_manager.rs
@@ -278,7 +278,9 @@ mod test {
         };
 
         let mut context = Context::new();
-        context.set_global_property_value(Parameters, p_values.clone()).unwrap();
+        context
+            .set_global_property_value(Parameters, p_values.clone())
+            .unwrap();
         context.init_random(p_values.seed);
 
         schedule_birth(&mut context);
@@ -300,7 +302,9 @@ mod test {
         };
 
         let mut context = Context::new();
-        context.set_global_property_value(Parameters, p_values.clone()).unwrap();
+        context
+            .set_global_property_value(Parameters, p_values.clone())
+            .unwrap();
         context.init_random(p_values.seed);
         let _person = context.create_new_person(0);
         schedule_death(&mut context);