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

UtilityFuncCRRA #1168

Merged
merged 26 commits into from
Jan 9, 2023
Merged

UtilityFuncCRRA #1168

merged 26 commits into from
Jan 9, 2023

Conversation

alanlujan91
Copy link
Member

@alanlujan91 alanlujan91 commented Aug 26, 2022

This PR creates an object oriented CRRA utility function to better organize namespaces and add formal structure to models.

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

fix failing errors
@codecov-commenter
Copy link

codecov-commenter commented Aug 26, 2022

Codecov Report

Base: 73.66% // Head: 73.38% // Decreases project coverage by -0.28% ⚠️

Coverage data is based on head (5dc1dc7) compared to base (ac76f68).
Patch coverage: 67.37% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1168      +/-   ##
==========================================
- Coverage   73.66%   73.38%   -0.29%     
==========================================
  Files          73       74       +1     
  Lines       12024    12150     +126     
==========================================
+ Hits         8858     8916      +58     
- Misses       3166     3234      +68     
Impacted Files Coverage Δ
HARK/ConsumptionSaving/ConsRiskyAssetModel.py 41.03% <0.00%> (ø)
HARK/ConsumptionSaving/ConsPrefShockModel.py 79.68% <50.00%> (ø)
HARK/utilities.py 24.57% <50.00%> (-5.53%) ⬇️
HARK/rewards.py 54.44% <54.44%> (ø)
HARK/ConsumptionSaving/ConsMarkovModel.py 85.79% <54.54%> (-0.05%) ⬇️
HARK/ConsumptionSaving/ConsMedModel.py 73.99% <72.22%> (-0.06%) ⬇️
HARK/ConsumptionSaving/ConsAggShockModel.py 89.64% <80.00%> (ø)
HARK/ConsumptionSaving/ConsGenIncProcessModel.py 82.50% <84.61%> (+0.05%) ⬆️
HARK/ConsumptionSaving/ConsIndShockModel.py 86.40% <100.00%> (-0.10%) ⬇️
HARK/ConsumptionSaving/ConsLaborModel.py 82.97% <100.00%> (+0.07%) ⬆️
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sbenthall
Copy link
Contributor

This looks like a good step forward to me!

@alanlujan91
Copy link
Member Author

To do from @llorracc: look for standardized function representation (with derivatives and inverses) in other python libraries

@alanlujan91
Copy link
Member Author

@alanlujan91
Copy link
Member Author

To do: Think of design for CRRA(CD) and/or CRRA(CES).

@sbenthall sbenthall mentioned this pull request Oct 11, 2022
3 tasks
@sbenthall
Copy link
Contributor

Looks like there's significant overlap between this PR and #1177

This looks like a simpler feature, and unless there's some good reason I'm not thinking of, it would be best to finish this one in its own PR.

@sbenthall
Copy link
Contributor

Fix merge conflict please.

@alanlujan91
Copy link
Member Author

The last discussion on this was that we need to settle on a standard before this is merged. Maybe I should close it for now.

@sbenthall
Copy link
Contributor

My bad ...

@sbenthall
Copy link
Contributor

@llorracc : advocates aligning this syntax with dolo's

@alanlujan91
Copy link
Member Author

Would like to have all econ utility functions in HARK.utilities, but I think it's useful for econ UtilityFunction objects to inherit from MetricObject so they can be compared, ie what is the distance between utility functions? However, when I add from HARK.core import MetricObject into HARK.utilities I get a circular reference error because core.py inherits from utilities.py.

I would like to propose that the file utilities.py contains only classes/methods that pertain to econ utility functions, and everything else that is more of a programming utility to go into a file called tools.py or something like that. This would avoid the circular reference.

@alanlujan91
Copy link
Member Author

TODO : labeled goods

@sbenthall sbenthall added this to the 0.13.0 milestone Jan 4, 2023
@sbenthall
Copy link
Contributor

@alanlujan91 See merge conflicts and test failures.

Punt the broader scope issues to #1035.
Lots of good stuff in this PR as is.

@alanlujan91 alanlujan91 changed the title [WIP] UtilityFuncCRRA UtilityFuncCRRA Jan 5, 2023
@alanlujan91 alanlujan91 added Status: Review Needed Ready-To-Merge Has been reviewed and when branch is updated and checks pass it should be merged labels Jan 5, 2023
@alanlujan91 alanlujan91 linked an issue Jan 5, 2023 that may be closed by this pull request
@alanlujan91
Copy link
Member Author

now addresses #1188

@alanlujan91 alanlujan91 removed their assignment Jan 6, 2023
@alanlujan91
Copy link
Member Author

alanlujan91 commented Jan 9, 2023

Changes in this PR:

  • Move econ utilities to rewards.py.
  • Rename parameter gam to rho in CRRAutility and derivatives.
  • Fix ZeroDivisionError in all utility functions.
  • Create cobb_douglas, cobb_douglas_p, cobb_douglas_pp, and cobb_douglas_pn functions.
  • Create const_elast_subs and const_elast_subs_p functions.
  • Implement abstract UtilityFunction class.
  • Implement UtilityFuncCRRA class with derivative and inverse.
  • Implement UtilityFuncCobbDouglas with derivative.
  • Implement UtilityFuncCobbDouglasCRRA.
  • Implement UtilityFuncConstElastSubs with derivative.
  • Replace instances of CRRAutility with UtilityFuncCRRA throughout codebase.

@sbenthall
Copy link
Contributor

I'll review this and merge if I don't see any issues.

@sbenthall
Copy link
Contributor

Great work! Merging.

@sbenthall sbenthall merged commit 881aa06 into econ-ark:master Jan 9, 2023
@alanlujan91 alanlujan91 linked an issue Feb 2, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready-To-Merge Has been reviewed and when branch is updated and checks pass it should be merged Status: Review Needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CRRAutility gives ZeroDivisionError support for curried utility functions
3 participants