search

econ-ark / HARK

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

Use Parameters class in solver #1389

Open Mv77 opened 2 months ago

Mv77 commented 2 months ago

I think that one of the main advantages of the Parameters class that @alanlujan91 has created is that, with its __get_item__ method, we can do away with a lot of ugly and repetitive code in HARK that determines what arguments to give to a solver in a given period given what is time-varying and what is not.

In an ongoing project, I have created an agent class that stores its parameters in a Parameters object. That parameter is then used by the solver to feed time-specific parameters to the solve_one_period method.

Although I am not yet sharing the agent prototype, I wanted to share and get comments on the solver. Currently what I am doing is forking the solve_one_cycle method. If the agent has a Parameters object, it takes advantage of it. If not, it uses the old and unchanged method.

mnwhite commented 2 months ago

Is there ugly and repetitive code that does that for solvers? It exists for the simulation code, and it's a problem that's getting fixed, but I don't think there's any issue for solvers, AFAIK.

On Thu, Feb 22, 2024 at 8:22 PM Mateo Velásquez-Giraldo < @.***> wrote:

I think that one of the main advantages of the Parameters class that @alanlujan91 https://github.com/alanlujan91 has created is that, with its __get_item__ method, we can do away with a lot of ugly and repetitive code in HARK that determines what arguments to give to a solver in a given period given what is time-varying and what is not.

In an ongoing project, I have created an agent class that stores its parameters in a Parameters object. That parameter is then used by the solver to feed time-specific parameters to the solve_one_period method.

Although I am not yet sharing the agent prototype, I wanted to share and get comments on the solver. Currently what I am doing is forking the solve_one_cycle method. If the agent has a Parameters object, it takes advantage of it. If not, it uses the old and unchanged method.

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

You can view, comment on, or merge this pull request online at:

https://github.com/econ-ark/HARK/pull/1389 Commit Summary

File Changes

(1 file https://github.com/econ-ark/HARK/pull/1389/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/econ-ark/HARK/pull/1389, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADKRAFJD5VGXED4ZVW4L4CLYU7VNHAVCNFSM6AAAAABDV57JK6VHI2DSMVQWIX3LMV43ASLTON2WKOZSGE2TAMRRGY4DCMA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 57.50000% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 71.45%. Comparing base (bbb07a5) to head (c11703b).

Files Patch % Lines
HARK/core.py 57.50% 17 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1389 +/- ## ========================================== - Coverage 71.53% 71.45% -0.08% ========================================== Files 83 83 Lines 13938 13955 +17 ========================================== + Hits 9971 9972 +1 - Misses 3967 3983 +16 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Mv77 commented 2 months ago

That's fair, I exaggerated. This is an instance where the Parameters tool that will eliminate a lot of ugly simulation code can also make solution code slightly more beautiful :)

            # Update time-varying single period inputs
            for name in agent.time_vary:
                if name in these_args:
                    solve_dict[name] = agent.__dict__[name][k]
            solve_dict["solution_next"] = solution_next

            # Make a temporary dictionary for this period
            temp_dict = {name: solve_dict[name] for name in these_args}

turns into 

            # Make a temporary dictionary for this period
            temp_pars = agent.params[k]
            temp_dict = {
                name: solution_next if name == "solution_next" else temp_pars[name]
                for name in these_args
            }
mnwhite commented 2 months ago

This is a step in the right direction, and also sets up a "conversion path" by maintaining backwards compatibility. Will look at promptly.

On Mon, Feb 26, 2024 at 9:56 AM Alan Lujan @.***> wrote:

@alanlujan91 https://github.com/alanlujan91 requested your review on:

1389 https://github.com/econ-ark/HARK/pull/1389 Use Parameters class

in solver.

— Reply to this email directly, view it on GitHub https://github.com/econ-ark/HARK/pull/1389#event-11922730967, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADKRAFIQCGCJOIF7PWTNS7DYVSPBJAVCNFSM6AAAAABDV57JK6VHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRHEZDENZTGA4TMNY . You are receiving this because your review was requested.Message ID: @.***>