search

econ-ark / HARK

Heterogenous Agents Resources & toolKit
Apache License 2.0
315 stars 195 forks source link

remove references to `parent_class_name` and some redundant constructions #1407

Closed alanlujan91 closed 1 month ago

mnwhite commented 1 month ago

I can't work out why, but one of the seemingly extraneous calls to update_solution_terminal is actually critical for exactly one test: the Jacobian methods on IndShockConsumerType. The answer isn't just a little off, it's off by orders of magnitudes. I didn't write those methods and don't really understand them, but their test is really simple. Very strangely, putting an additional call to update_solution_terminal into the test itself doesn't fix the issue.

Anyway, all tests pass after resurrecting that one line, so...?

alanlujan91 commented 1 month ago

@wdu9 I believe the jacobian work was all done by you. Do you have any guesses as to why this is critical?

For a summary of what this PR does: We found there are numerous calls to update, update_income_process, or update_solution_terminal for a a few models, when obviously you should only have to do this once. To correct for this I removed the superfluous calls to ensure we only update everything once. Every other test in HARK works, except for test_Jacobian_methods. https://github.com/econ-ark/HARK/actions/runs/8584487477/job/23524783772

mnwhite commented 1 month ago

Per meeting today, I'm merging this, then will try to get @wdu9 to look into the mysterious maybe-bug.