Skip to content
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

Implemented first pass at generalizable compartmental model #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kabirmoghe
Copy link
Collaborator

No description provided.

def __init__(
self,
compartments: List[str],
states: Dict[str, float],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you don't need this param, which is nice! Less params == :)

compartments: List[str],
states: Dict[str, float],
parameters: Dict[str, float],
transitions: List[Dict],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love how you designed the notion of a transition. It feels like a great way of thinking about a compartmental model.

Actually writing down the transitions is going to be difficult no matter how you implement it. Ideally, you'd like a programmer to be able to understand how to create the right argument to pass to transitions just by reading the documentation. It took me a few examples to understand how the transitions data needed to be structured. I might add some examples to the docs and make the argument description super clear and specific.

You might be able to accept an adj matrix of transition functions instead of a dictionary. But I don't know which is clunkier.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely a bit unintuitive, so I'll add some detailed documentation for that + examples. I think an adjacency matrix would be tricky given the nature of some of the transitions, right?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You would be the expert on this rather than me! I mainly gave the adj matrix as an contrasting example to help illustrate how the form of parameters can make the function easier or harder to use. As long as you've thought about it, I trust your judgement.

def drift(self, x: np.ndarray, ti: float):
"""Deterministic component of the compartmental model"""
# Defines each state and derivative
current_states = dict(zip(self.compartments, x))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the way interfere is designed, you're going to have to assume that the order of the variables in x corresponds to the order of variables in self.compartments.

At the same time, this assumption might let you streamline your transition functions by writing them as:

I_to_R = lambda S, I, R, beta, sigma, gamma: gamma * I * R

then when you call them it could look like this

for compartment_idx in range(self.dim)
    for f in self.transition_adj[component_idx, :]:
        dX[compartment_idx] += f(*x, **self.parameters)

as long as parameters is a dict that has all the correct param names.

I'm not saying that you have to do this, just that it is something to think about. The main goal is to streamline the code and make it easy for other people to use.

Comment on lines +70 to +79
if self.scalar_noise:
noise *= rate

rate += noise

# Modifying states using rate
if prev_state is not None:
derivatives[prev_state] -= rate
if next_state is not None:
derivatives[next_state] += rate
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's tricky to see if noise implemented this way can be expressed in the form of a standard SDE which is:

dX = mu(x, t)dt + sigma(x, t) dW

see here for details .

I can probably write the noise functions for you. I just need to know if people model the noise on the transition level or if they usually model it at the compartmental level. (You are doing it at the transition level here) I can even use the parameter scalar noise to toggle between additive and multiplicative noise inside the noise function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's a good question to ask. I'm honestly not sure, but intuitively, it seemed like keeping it at the transition level was at least a decent way of preserving the system's total population. Does the way it's written now seem like it makes sense?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good justification for modeling it at the transition level. For the scope of work here it's worth it to me to have the model conform to the standard SDE format even if there are fluctuations in the total population.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants