-
-
Notifications
You must be signed in to change notification settings - Fork 199
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
Add IncShkDstn constructor for ConsMarkov #1484
base: master
Are you sure you want to change the base?
Conversation
DWC pointed out that MarkovConsumerType doesn't work "off the shelf". This PR fixes that with a new custom income process constructor. Tests run correctly, but example notebooks might break, let's find out!
This addresses #1478 |
Adjustments for new constructor.
Turn off default Markov constructor in test.
One test is failing on Ubuntu, Py 3.10. It runs fine on other OS and versions of Python. This is going to be a tedious process to fix.
This has one failing test, but only on Ubuntu. It runs fine on Windows and Mac. Any suggestions on how to test this locally? |
You can use pytest, assuming you have a linux system. Check the contributor guide. |
I don't, or at least not much of one. I was hoping there was some easy way
to run a Linux instance remotely or something.
If I have to make a local installation... ugh, fine.
…On Fri, Aug 16, 2024, 9:51 PM DominicWC ***@***.***> wrote:
You can use pytest, assuming you have a linux system. Check the
contributor guide.
—
Reply to this email directly, view it on GitHub
<#1484 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADKRAFLRGZQUYHXFOCDUDTTZR2UBHAVCNFSM6AAAAABMURP4FOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJUGUZDKOJYHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Following @alanlujan91 's advice, I set up WSL on one of my computers, got Miniconda set up, installed HARK locally, and ran the failing test... and it didn't fail. Now what? |
Ok, super fascinating result: I re-ran the Ubuntu Py 3.11 tests remotely... and it succeeded. And then I ran all consumption tests locally (rather than just the failing one)... and it failed. Something is going on with the order of tests on Linux, where test A is affecting test B. |
Have you fixed the test that runs in Compare_TBS_and_Markov? Maybe PermShkStd and TranShkStd's shape haven't been updated. |
I don't know what to fix because I can't consistently reproduce the error
locally.
…On Mon, Aug 19, 2024 at 3:02 PM DominicWC ***@***.***> wrote:
Have you fixed the test that runs in Compare_TBS_and_Markov? Maybe
PermShkStd and TranShkStd's shape haven't been updated.
—
Reply to this email directly, view it on GitHub
<#1484 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADKRAFPQ4AMHB3ALUDLBTMTZSI6N3AVCNFSM6AAAAABMURP4FOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJXGI2DGNJSGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
A dictionary might be changed in an unexpected way.
I managed to fix this without fully understanding why. I switched one dictionary line in an init method from copy to deepcopy, and the error went away. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. It fixes the problem, passes checks, and it's inputs are consistent with the rest of the ConsMarkovModel. I couldn't find any issues.
DWC pointed out that MarkovConsumerType doesn't work "off the shelf". This PR fixes that with a new custom income process constructor. Tests run correctly, but example notebooks might break, let's find out!
Please ensure your pull request adheres to the following guidelines: