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

namespace for control variables #855

Merged
merged 4 commits into from
Oct 29, 2020
Merged

namespace for control variables #855

merged 4 commits into from
Oct 29, 2020

Conversation

sbenthall
Copy link
Contributor

@sbenthall sbenthall commented Oct 21, 2020

This puts each 'control' variable into a namespace, controls, similar to shocks.

It does not adjust the storage to use '_now' or '_prev' dictionaries for the controls.
We can discuss this choice in the meeting -- fore example, whether we prefer to put all the 'model variables' including shocks and controls into 'now' and 'prev' dicts.

This is passing tests locally.

  • 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
Copy link
Contributor Author

This PR includes all the commits from i836, which should be merged FIRST.

@sbenthall sbenthall requested a review from mnwhite October 23, 2020 01:11
@sbenthall
Copy link
Contributor Author

I just saw the merge conflicts and have pushed the resolution.

Requesting review and merge from @mnwhite

After this PR, I will start working on cutting the 0.10.8 release.

@sbenthall
Copy link
Contributor Author

Before cutting the release, I will fix the DemARKS!

@sbenthall
Copy link
Contributor Author

Reminder: @mnwhite request for review

@mnwhite
Copy link
Contributor

mnwhite commented Oct 29, 2020

This is exactly analogous to prior PR, and local tests pass. This should be good for merging.

@mnwhite mnwhite merged commit 866f78e into econ-ark:master Oct 29, 2020
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