-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Specify rate functions in input.json
#62
base: main
Are you sure you want to change the base?
Conversation
@@ -4,6 +4,12 @@ use std::path::PathBuf; | |||
use ixa::{define_global_property, ContextGlobalPropertiesExt, IxaError}; | |||
use serde::{Deserialize, Serialize}; | |||
|
|||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@k88hudson-cfa what do you think of this approach for specifying in different rate functions?
/// Instantiate the rate functions specified in the global parameter `rate_of_infection` as actual | ||
/// rate functions for the simulation that are assigned randomly to agents when they are infected. | ||
pub fn instantiate_rate_fns(context: &mut Context) -> Result<(), IxaError> { | ||
let rate_of_infection = context.get_params().rate_of_infection.clone(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really don't love that this structure basically duplicates the info in rate_of_infection
. I think there are two potential solutions: (a) implement InfectiousnessRateFn
on enum Rates
instead, or (b) actually move the info in rate_of_infection
rather than copying it. I don't think (a) is a possible option because in implementing the methods for InfectiousnessRateFn
on EmpiricalRate
, you need to create new vectors that you store in the initialization process, and I don't see a way to do this if we just implement the method on the type as it is fed in from deserialization. I think (b) is an easy option, but I am open to other thoughts and ways of thinking about this.
I think we have to move the info (at least) because in EmpiricalRate
we want to create
Rates::Constant(rate) => Box::new(ConstantRate::new(rate, infection_duration)?), | ||
Rates::Empirical(rate_fn) => { | ||
let (t, r): (Vec<f64>, Vec<f64>) = rate_fn.into_iter().unzip(); | ||
Box::new(EmpiricalRate::new(t, r)?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, doesn't using Box
create an overhead here/should we be concerned about that? I could imagine us having a number of infectiousness curves on the order of the number of agents in the simulation.
WIP