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

Plotting graph of equations for FramedAgentType #1071

Merged
merged 18 commits into from
Nov 18, 2021

Conversation

sbenthall
Copy link
Contributor

@sbenthall sbenthall commented Oct 12, 2021

This PR introduces into FramedAgentType the mechanics for frames to reference each other as 'parents' and 'children', including 'back' and 'forward' references when these frames are in a different period.

This enables the plotting of a MACID style network diagram representing the problem, as in the following case (taken from the example notebook):

image

  • 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.

@project-bot project-bot bot added this to Needs Triage in Issues & PRs Oct 12, 2021
@sbenthall
Copy link
Contributor Author

This builds on #1064, which should be merged first.

@codecov-commenter
Copy link

codecov-commenter commented Oct 13, 2021

Codecov Report

Merging #1071 (c6b7dd0) into master (a5634ab) will increase coverage by 0.04%.
The diff coverage is 89.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1071      +/-   ##
==========================================
+ Coverage   73.60%   73.64%   +0.04%     
==========================================
  Files          69       69              
  Lines       10519    10576      +57     
==========================================
+ Hits         7742     7789      +47     
- Misses       2777     2787      +10     
Impacted Files Coverage Δ
HARK/frame.py 92.42% <84.78%> (-4.09%) ⬇️
HARK/ConsumptionSaving/ConsPortfolioFrameModel.py 95.23% <100.00%> (-4.77%) ⬇️
...mptionSaving/tests/test_ConsPortfolioFrameModel.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5634ab...c6b7dd0. Read the comment docs.

@alanlujan91 alanlujan91 self-requested a review November 5, 2021 18:17
@sbenthall sbenthall mentioned this pull request Nov 15, 2021
3 tasks
Copy link
Member

@alanlujan91 alanlujan91 left a comment

Choose a reason for hiding this comment

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

This looks good to me. Is there a way to provide a pseudo-simulation history of shocks that can be used by both agent types? In this way, they both experience the same exact shocks, and differences would be fully attributable to bugs and not to changes in random draws.

@sbenthall
Copy link
Contributor Author

That's an excellent idea.

I believe there is support for this in the library, but I've never used it:

HARK/HARK/core.py

Lines 553 to 556 in a5634ab

if self.read_shocks: # If shock histories have been pre-specified, use those
self.read_shocks_from_history()
else: # Otherwise, draw shocks as usual according to subclass-specific method
self.get_shocks()

Let me try that out before merging. Thanks for the review!

@sbenthall
Copy link
Contributor Author

Ok, I tried copying the shock history over from the first portfolio simulation to the Framed version and using read_shocks.

The results were somewhat unexpected:

  • The shock values did not line up exactly.
  • But the results of the simulation (i.e, derived values, such as mNrm) did come into closer alignment.

I may well be using read_shock incorrectly and so this needs more investigation.

But since that testing is somewhat orthogonal to the plotting work here, I'll merge this PR and address this in a different issue.

@sbenthall sbenthall merged commit 760df61 into econ-ark:master Nov 18, 2021
Issues & PRs automation moved this from Needs Triage to Done Nov 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Issues & PRs
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants