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

Refactor checkConditions() code #542

Closed
sbenthall opened this issue Feb 24, 2020 · 6 comments
Closed

Refactor checkConditions() code #542

sbenthall opened this issue Feb 24, 2020 · 6 comments
Assignees
Milestone

Comments

@sbenthall
Copy link
Contributor

The checkConditions() code is a bit messy.

Here's one reason:
525cb7e#commitcomment-37457605

There is a lot more that could be done to streamline this code.

@sbenthall
Copy link
Contributor Author

@sbenthall sbenthall added this to the 0.x.y milestone Mar 11, 2020
@sbenthall
Copy link
Contributor Author

This issue is also related: #283

@sbenthall
Copy link
Contributor Author

Based on a chat with @llorracc last week, my understanding is that there are three classes of conditions:

  • Conditions that must be met for a model to have non-degenerate solutions
  • Conditions that must be met for the model to have a steady state distribution when solved and simulated for an indefinite amount of time
  • Conditions that are worth noting but have no direct implications for the functionality of software

In the current code, there is only one method for testing for the violation of these conditions.

But since one set of tests matters for solving the model, and another set of tests matters for simulating it, it follows that these tests should be handled separately and executed at different times.

@llorracc
Copy link
Collaborator

Based on a chat with @llorracc last week, my understanding is that there are three classes of conditions:

  • Conditions that must be met for a model to have non-degenerate solutions
  • Conditions that must be met for the model to have a steady state distribution when solved and simulated for an indefinite amount of time
  • Conditions that are worth noting but have no direct implications for the functionality of software

In the current code, there is only one method for testing for the violation of these conditions.

Right; the original code, though messy, did give varying responses attuned to these varying conditions.

But since one set of tests matters for solving the model, and another set of tests matters for simulating it, it follows that these tests should be handled separately and executed at different times.

I don't agree with that. People using these kinds of models are almost always going to want to simulate them, and it would be best for them to be forewarned at the time when they are solving the model if there will be problems simulating it. No point in working hard on calibrating or solving the solution part only to find that it is useless for your purposes when you get to the sim part.

@sbenthall
Copy link
Contributor Author

did give varying responses attuned to these varying conditions.

Do you mean varying responses in the print statements?
If so, I believe these statements are still there in #568 in its current state.

People using these kinds of models are almost always going to want to simulate them, and it would be best for them to be forewarned at the time when they are solving the model if there will be problems simulating it.

I don't disagree with this.

But is it always a problem if a simulation does not have a steady-state distribution?

I would propose that:

  • before running the solver, the user is warned if the result is known to be degenerate.
  • before running the simulation, the user is warned if conditions are such that there is no ergodic distribution

Even if you disagree and believe both checks should happen before running the solver, I think the main thing I'm getting at is that the two checks for model properties (nondegeneracy and ergodicity) should be separated in the code so it's possible to call one without the other. A user who knows what they are doing might want to calibrate a model and then test for one property, then the other. A designer of a dashboard might want to prevent a user from selecting values that lead to degeneracy, but not non-ergodicity. They can easily be different functions.

@sbenthall
Copy link
Contributor Author

Has this been resolved?

@MridulS MridulS closed this as completed Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants