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

Make calcChoiceProbs more accurate, and make simultaneous evalution f… #242

Merged
merged 1 commit into from
Apr 10, 2019

Conversation

pkofod
Copy link
Contributor

@pkofod pkofod commented Mar 19, 2019

…aster (and more accurate).

The commit message should say it. It makes the simultaneous evaluation faster and more accurate, and the choice prob evaluation more accurate on its own.

Improvements where needed for econ-ark/REMARK#9 to fully replicate DCEGM paper.

@pkofod
Copy link
Contributor Author

pkofod commented Apr 5, 2019

@mnwhite would it be possible for you to look at these changes? It'd probably make sense for them to make it into HARK before the next version, as the way they're calculated here is quite important to be able to replicate the DCEGM paper for example. With the old way of handling overflow, DCEGM figures do not match those published (and the fault is ours)

cc @shaunagm as I feel like this is slightly release blocking

@llorracc
Copy link
Collaborator

llorracc commented Apr 5, 2019 via email

@pkofod
Copy link
Contributor Author

pkofod commented Apr 5, 2019

Yes, I think these should be in the release -- but I thought they'd already been merged into the master branch?

Some version of the collection of functions are there, but this PR hasn't been merged, so the improvements are not. Edit: so the things we discussed with Shujaat are not in the master branch until this is merged.

@mnwhite
Copy link
Contributor

mnwhite commented Apr 5, 2019 via email

@mnwhite
Copy link
Contributor

mnwhite commented Apr 10, 2019

The loop at 3395 can be done with an indexing scheme (and no loop). But (from my own experience with this exact step) it makes the code harder to read and doesn't actually result in a speedup-- the number of choices is usually like 2-5, so it's not much of a loop to avoid.

Merging now.

@mnwhite mnwhite merged commit c394d0f into econ-ark:master Apr 10, 2019
@shaunagm
Copy link
Contributor

@pkofod @mnwhite - can one of you suggest a one line summary for the release notes?

@pkofod
Copy link
Contributor Author

pkofod commented Apr 11, 2019

"Improve accuracy and performance of functions for evaluating the integrated value function and conditional choice probabilities for models with extreme value type I taste shocks."

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.

4 participants