namespace for states; transition function for #761

[sbenthall]

Addresses #761

Still working on this.

• Tests for new functionality/models or Tests to reproduce the bug-fix in code.
• Updated documentation of features that add new functionality.
• Update CHANGELOG.md with major/minor changes.

[sbenthall]

Tests for this PR are passing locally.

The implementation is still a bit rough, especially in light of the dealing with "poststates", which are slated to be substantially revised (see #798).

But I think it is at a state now where it makes sense to request a review from @mnwhite .

[sbenthall]

Next steps for this PR:

• replace "post state" storage with a step at the beginning/end of a period which stores values from states_now in states_prev, and put None/NaN values in all the states_now
• include a configurable ante_vars with functions for initializing the state at time = 0.

[sbenthall]

Ah, working on this reminded me of the other reason for the poststate designation.

The poststates notionally need to be computed after a control variable is determined via a decision rule.

I believe that this wrinkle will get smoothed over once we use the topological order of model variables, or some user-defined framing, to determine execution order.

In the short term, I'm going to have to keep using the somewhat vague and overloaded 'poststate' category:

• getPoststates will calculate state values that depend on controls....
• ... but store these values in state_now momentarily...
• ... before transferring them to state_prev

[sbenthall]

I have almost all the bugs worked out in the new implementation (i.e., most tests are passing) but I have a sticky issue I need feedback from @mnwhite about.

It has to do with how the AgentType classes, now organized with state_now and state_prev namespaces, interact with the Market class.

Some things I notice:

• While the market solve() method uses agent simulation as part of the makeHistory() step, I don't believe initializeSim on the agents is ever called. This makes for a different path for seeding the antecedent state (i.e., the state_prev values needed to simulate a period.)
• I guess the pushing and pulling of state from the market to the agent objects is done in the sow() and reap() methods.
  • these used to work by setting agent attributes
  • now, we need to make the decision of whether they sow to state_prev and reap from state_now, or what.
  • It looks like the marketAction command simulates one period, which would cycle state_prev on time period--so maybe the market reads from and writes to state_prev?
    • does this use the same mortality mechanics as a regular simulation?

Thanks in advance for any thoughts on this.

[sbenthall]

Tests are now passing.

There remains a design issue which has not be resolved in a satisfactory way yet.

Model classes currently use a combination of explicitly defined state variables and inherited state variables.

In the current pattern, the lists of 'state' variables are inherited in such a way that many downstream classes have dummy states used by ancestor's models, but unused themselves. This is messy.

There is also a problem that in some places, the 'state variable' is defined by assignment to two dictionaries (state_now and state_prev) in the model constructors. This is untidy and can lead to bugs in future models.

[sbenthall]

There seem to be some inconsistencies across the model classes in how inheritance is done. This makes it harder to have a consistent design for these things.

I'm wondering if there's a specific reason why the constructor for AggShockConsumerType calls the AgentType constructor, rather than its superclass IndShockConsumerType's constructor. It takes several steps to reproduce some of the aspects of IndShockConsumerType after the call to the AgentType constructor.

[sbenthall]

Latest commit cleans up some of the state_var inheritance issues.

However, locally this change breaks the Fast models developed by @alanlujan91

@alanlujan91 do you have any insight into why this may be happening? I have not touched the Fast classes directly in this PR. The main thing the most recent change does to the core classes is create a state_vars (no underscore) variable on the AgentType class and has this variable be modified directly in subclasses like ConsIndShockModel

[sbenthall]

Progress: More testing has confirmed that before preSolve(), the AFunc that is being given to the KS agent has changed in this PR.

[sbenthall]

Ok. I figured out the problem.

I had been building out the test suite in an incorrect way.

The way it had been organized, the new tests I was writing (see #850) were running test_economy first, then later test_methods, using the same Economy object.

This was causing it to start with an already converged AFunc value, which was being distributed to the agent when getEconomyData was called.

I believe that the way reset() is written on the KS economy class and the Market class more generally, reset() will not change AFunc back to its original value! This seems like a bug to me.

What I'm doing now is rewriting the test cases in the recommended way: the setUp function is defined in one TestCase class, which is then subclassed for each more specific test.

Separately, I wonder if @mnwhite agrees that this behavior of reset() is not ideal. If so, I'll add an issue for it. This seems related, but not identical, to #849.

[sbenthall]

Nope, it's not that easy...(!)

Now I've isolated the problem to updateDynamics().
AFunc is not being set the same value across the two branches.

Worse, it looks like there's some stochastic variation in updateDynamics such that running the same test multiple times on the same branch comes up with different AFunc[0].slope values.

[sbenthall]

The calculation of the AFunc seems to depend on some attributes of the market, intercept_prev and slope_prev. These are as far as I know neither sow or reap variables on the market, but it looks like they are neverthless some kind of stored Market state that is iterated on.

What's happening here?

[sbenthall]

The KS economy class had a bug.
It used a copy() of the parameters dictionary, instead of a deepcopy().
Because the economy class stores the previous slope/intercept by mutating the lists, this was changing the default intercept and slope values.

This bug is now fixed in this PR.

[sbenthall]

There was an error in my transitioning of KS into the new state namespace.
I didn't properly change mNow. Tests are passing locally now!

[sbenthall]

ugh. One more case to fix: the Portfolio model.

[sbenthall]

The tricky issue here seems to be the handling of RiskyNow.
I guess this should be treated like a shock, except that it's an aggregate shock (i.e., it's the same for all agents).

[sbenthall]

Ok. Tests are passing locally. Phew

The issue was with how I was dealing with initialization and birth of AdjustNow and ShareNow.
In the current Portfolio model, these are anomalous: they need to be set as "post states" for a period's transition function, but they are a shock and a control variable respectively. As such they are not handled yet in this new treatment of state_now and state_prev.

I'll need to revisit this design while working on #838 (namespacing the control variables).

Please merge #853 , new tests for ConsPortfolio model, before this PR.