Support multiple regimes per model #166
Conversation
…group (#167) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
| state_action_spaces = build_state_action_spaces(regime) | ||
| next_state = get_next_state_function( | ||
| internal_functions=internal_functions, | ||
| transitions=flatten_to_qnames(internal_functions.transitions), |
There was a problem hiding this comment.
Can you write a wrapper function for the flatten_to_qnames functions in some utils module that explains what behavior we expect in lcm. Since transitions have a specific structure we can say more about the expected structure of the outcome that way.
|
|
||
| internal_transition = { | ||
| fn_name: functions[fn_name] for fn_name in regime.transitions | ||
| fn_name: functions[fn_name] for fn_name in flatten_to_qnames(regime.transitions) |
There was a problem hiding this comment.
Same here and for the unflattening part. Would be better if these functions come from some of our modules and have a name that potentially add some information of what happens in our case.
src/lcm/model.py
Outdated
| argmax_and_max_Q_over_a_functions=self.internal_regimes.argmax_and_max_Q_over_a_functions, | ||
| next_state_simulation_function=self.internal_regimes.next_state_simulation_function, | ||
| internal_regime=self.internal_regimes, |
There was a problem hiding this comment.
I like that you only pass the internal regimes to solve and not the individual functions. Can we do the same for simulate?
src/lcm/regime.py
Outdated
| functions: dict[str, UserFunction] = field(default_factory=dict) | ||
| actions: dict[str, Grid] = field(default_factory=dict) | ||
| states: dict[str, Grid] = field(default_factory=dict) | ||
| regime_transition_probabilities: Callable[..., dict[str, float]] |
There was a problem hiding this comment.
regime_transition_probs, otherwise it is too long.
src/lcm/model.py
Outdated
| ) | ||
| self.regimes[regime.name] = regime | ||
| self.internal_regimes[regime.name] = process_regime( | ||
| regime=regime, n_periods=n_periods, enable_jit=enable_jit |
There was a problem hiding this comment.
Don't we have to update the regime processing to include all regimes? I thought that we need to do the processing of certain regime components first, as we then need to pass the processed components of all regimes into each regime? I.e., the Q-and-F function of a regime depends on all of the other regimes?
for more information, see https://pre-commit.ci
This PR is supposed to add features to handle Models that consist of multiple deterministic Regimes.
ModelandRegimeobjectsFor the next PR(s)