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

Update PerfForesightConsumerType #1358

Merged
merged 1 commit into from
Nov 15, 2023
Merged

Conversation

sidd3888
Copy link
Collaborator

@sidd3888 sidd3888 commented Nov 10, 2023

Made some fixes in the language and description of the model and the code. One mistake left to be clarified.

Paragraph in the subsection Example parameter values to construct an instance of PerfForesightConsumerType that elaborates, "In contrast, we could specify a life-cycle model...", which states that the parameter T_cycle could be set to $1$. However, it is already set to $1$ and this would not allow room for age-varying parameters of income growth and survival probability.

Ready to merge in all other aspects.

CC: @alanlujan91

Made some fixes in the language and description of the model and the code. One mistake left to be clarified
@mnwhite
Copy link
Contributor

mnwhite commented Nov 10, 2023 via email

Copy link

codecov bot commented Nov 10, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3ec3404) 73.02% compared to head (c70ebc9) 73.02%.
Report is 8 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1358   +/-   ##
=======================================
  Coverage   73.02%   73.02%           
=======================================
  Files          79       79           
  Lines       13377    13377           
=======================================
  Hits         9769     9769           
  Misses       3608     3608           

see 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alanlujan91 alanlujan91 self-assigned this Nov 15, 2023
@alanlujan91 alanlujan91 merged commit 2ad4e89 into econ-ark:master Nov 15, 2023
18 checks passed
@alanlujan91 alanlujan91 assigned mnwhite and unassigned alanlujan91 Nov 15, 2023
@sidd3888
Copy link
Collaborator Author

@mnwhite The thing I pointed out, and you were looking into, is still pending to be resolved. Will open a new PR to resolve that once these stylistic issues have been decided upon. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants