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

[WIP] Updates to code #4

Merged
merged 34 commits into from
Apr 21, 2023
Merged

[WIP] Updates to code #4

merged 34 commits into from
Apr 21, 2023

Conversation

alanlujan91
Copy link
Member

No description provided.

@alanlujan91
Copy link
Member Author

alanlujan91 commented Jan 2, 2023

@llorracc @wdu9 @dedwar65 cstwMPC.ipynb still runs in master branch, results are slightly different

@alanlujan91
Copy link
Member Author

I was wrong... the repo doesn't work on master branch yet

@alanlujan91
Copy link
Member Author

some changes are required on HARK source code to make this run

@llorracc
Copy link

llorracc commented Jan 2, 2023

I believe Seb had pinned it to whatever version of HARK he was using when he updated it, so it should still work with that version, right? So I guess you're saying that it is incompatible with subsequent changes to HARK.

@alanlujan91
Copy link
Member Author

yes, @sbenthall pinned this to version 0.11

This is part of the work to bring it up to date with development HARK

@sbenthall
Copy link

The 'logging error' is actually due to invalidly specified DiscFac, which is being set somewhere to an np.array (rather than a float or a list, which are what HARK accepts in its API).

I have not been able to test this further on this PR because of a local error AttributeError: 'Uniform' object has no attribute 'approx', which seems to be due to the recent merge of econ-ark/HARK#1197

@llorracc
Copy link

@sbenthall, since @alanlujan91 will be otherwise occupied in the next few days, could you take charge of bringing the update of DistributionOfWealthMPC over the finish line of compatibility with the current development branch? Alan may have enough bandwidth to consult on how to bring his tools for distributing parameters to bear. And Will Du may be able to advise about how to revamp it otherwise (esp to speed it up). But we need a ringmaster.

@sbenthall
Copy link

I can commit to continuing the work here to bring this repo up to HARK 0.13.0

That will be much easier after the 0.13.0 release, since then it will no longer be a moving target.

It would be sensible to first merge a PR updating the dependency to 0.12.0. I'll start with that.

It would be a good idea to make separate issues for the other improvements you have mentioned. The details can be worked out in those issues.

@sbenthall
Copy link

See #5 for a discussion of population improvements.

@sbenthall
Copy link

@wdu9 Could you please make an issue in the repository describing the performance improvements that you have in mind?

@alanlujan91
Copy link
Member Author

this PR should now bring this project up to date with master branch

@alanlujan91 alanlujan91 requested a review from wdu9 January 20, 2023 01:09
@alanlujan91
Copy link
Member Author

can't seem to be able to tag you but can you review this as well @dedwar65?

@dedwar65
Copy link
Collaborator

dedwar65 commented Jan 20, 2023 via email

@llorracc
Copy link

llorracc commented Jan 20, 2023 via email

@dedwar65
Copy link
Collaborator

dedwar65 commented Jan 20, 2023 via email

@dedwar65
Copy link
Collaborator

dedwar65 commented Jan 23, 2023 via email

@alanlujan91
Copy link
Member Author

@dedwar65 this is an error because the project you are working on should not be on the master branch, but instead should probably be pinned to econ-ark = 0.12. .approx is deprecated in the master branch and will not be available in econ-ark = 0.13. It has been replaced by .discretize(N, method). We should probably have a deprecation warning in master branch for this.

@llorracc
Copy link

llorracc commented Jan 23, 2023 via email

@alanlujan91 alanlujan91 changed the title [WIP] Updates to use econ-ark/HARK@master development branch [WIP] Updates to code Mar 1, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@alanlujan91 alanlujan91 linked an issue Apr 12, 2023 that may be closed by this pull request
@alanlujan91 alanlujan91 merged commit 57d03ea into econ-ark:master Apr 21, 2023
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.

TypeError: multiple values for 'reap_var' with do_mid.py
4 participants