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

Binchengecon submit deepaiygari #1281

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

binchengecon
Copy link

Please ensure your pull request adheres to the following guidelines:

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

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov
Copy link

codecov bot commented Jun 14, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (37134b9) 72.55% compared to head (dc64074) 72.55%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1281   +/-   ##
=======================================
  Coverage   72.55%   72.55%           
=======================================
  Files          78       78           
  Lines       13009    13009           
=======================================
  Hits         9439     9439           
  Misses       3570     3570           

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

@llorracc
Copy link
Collaborator

llorracc commented Jun 14, 2023 via email

@binchengecon
Copy link
Author

binchengecon commented Jun 14, 2023

Small comments: 1. Please rename to Aiyagari-Deep - This is to match our existing Aiyagari-HARK and Aiyagari-Dolo REMARKs 2. As you continue, please do your best (I know it’s hard) to look for patterns like this to match On Tue, Jun 13, 2023 at 9:51 PM codecov[bot] @.*** @.> wrote: Codecov
https://app.codecov.io/gh/econ-ark/HARK/pull/1281?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=econ-ark Report Patch and project coverage have no change. Comparison is base (37134b9) https://app.codecov.io/gh/econ-ark/HARK/commit/37134b9c9eea4591659b7cc398cf5894e44ba0e7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=econ-ark 72.55% compared to head (dc64074) https://app.codecov.io/gh/econ-ark/HARK/pull/1281?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=econ-ark 72.55%. Additional details and impacted files @@ Coverage Diff @@## master #1281 +/- ## ======================================= Coverage 72.55% 72.55% ======================================= Files 78 78 Lines 13009 13009 ======================================= Hits 9439 9439 Misses 3570 3570 ☔ View full report in Codecov by Sentry https://app.codecov.io/gh/econ-ark/HARK/pull/1281?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=econ-ark . 📢 Do you have feedback about the report comment? Let us know in this issue https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=econ-ark . — Reply to this email directly, view it on GitHub <#1281 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKCK77S7Z7F7JNS2W7TIGLXLEKIXANCNFSM6AAAAAAZFOASLQ . You are receiving this because you are subscribed to this thread.Message ID: @.
>
-- - Chris Carroll

Hello,

I did two things:

  1. Submit a new PR named as "Aiyagari-Deep", mimicking the style in "Aiyagari_Idiosyncratic".
  2. Add requirement file for package installation.

@binchengecon
Copy link
Author

binchengecon commented Jun 21, 2023 via email

@sbenthall
Copy link
Contributor

Hello @binchengecon

Thanks for this PR!

We have looked over this submission and we think it would be better as a REMARK, rather than a contribution to the HARK library.
https://github.com/econ-ark/REMARK

REMARKS are published research products, whereas the HARK examples are demonstrations about how to use our HARK toolkit.

What would be especially welcome is a comparison between your deep learning Aiyagari model and the existing REMARK implementation, which is here:
https://econ-ark.org/materials/aiyagari

We are interested to know how we could document better these guidelines for contributors to submit REMARKs. Maybe we need to update the contribution guide.
https://docs.econ-ark.org/guides/contributing.html

cc @DominicWC

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.

3 participants