-
-
Notifications
You must be signed in to change notification settings - Fork 198
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
Equations in Models #1292
base: master
Are you sure you want to change the base?
Equations in Models #1292
Conversation
Linking to Dolo documentation about the Model for inspiration here: |
Also for reference... I different way to implement a representation of mathematical equations has already been committed in the The equations get evaluated in simulation in the scope of the current state/shock/parameter values: https://github.com/econ-ark/HARK/blob/master/HARK/frame.py#L591-L670 At the time I wrote that, I had no idea how to integrate equations like these into the solver. #1283 provides a way to do that. But also, there are pros and cons to the implementation in |
The failing tests in the ConsAggShockModels are due to the fact that the AggShockModel constructor does not call the constructor of its superclass ( https://github.com/econ-ark/HARK/blob/master/HARK/ConsumptionSaving/ConsAggShockModel.py#L135-L156 This is not how other model inheritance in HARK works, where the superclass is called. Is this intentional? Or just an oversight in how the ConsAggShockModel works? if the latter, I can easily fix it in this PR. |
That looks like an oversight, but there might be some initialization
routine for IndShockConsumerType that's not supposed to be run for an
AggShockConsumerType. Let's try changing the inheritance and see if it runs
properly.
…On Wed, Jul 5, 2023 at 10:58 AM Sebastian Benthall ***@***.***> wrote:
@sbenthall <https://github.com/sbenthall> requested your review on: #1292
<#1292> [WIP] Equations in Models.
—
Reply to this email directly, view it on GitHub
<#1292 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADKRAFOY5HTVAAWDCWQYCM3XOV6JNANCNFSM6AAAAAAZWHG3XU>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
Ok, I changed it locally and it got tests passing. This change has been committed to this PR. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1292 +/- ##
==========================================
+ Coverage 72.55% 72.61% +0.06%
==========================================
Files 78 80 +2
Lines 13009 13038 +29
==========================================
+ Hits 9439 9468 +29
Misses 3570 3570
☔ View full report in Codecov by Sentry. |
Tests are now passing. KinkedR had a similar issue with inheritance. The substance of this PR is now in and ready for review. Requesting review from @mnwhite I know I need to do tests, docs, and changelog, and will start on that next. |
Not sure why this just popped up in my inbox, but it prompted me to look again. I think thinking about all this is premature. It will really need to be integrated with whatever structure gets created from Pablo's architecture. At most, at this point we could perhaps fruitfully build on what is already in dolo or nolib. |
It's unfortunate that you've been missing the regular HARK meetings @llorracc I think we're quite resolved that this is a good idea. Essentially, no matter what Pablo's architecture is, the HARK models will need to be moved in this sort of direction in order to integrate with that. This is a necessary incremental improvement to HARK no matter what. |
I'd like to revisit this PR, which is now almost a year old, and perhaps out of date with respect to recent HARK changes. But it was trying to accomplish something that we are still trying to move towards. The main point of this PR was to try attaching dynamics to the existing ConsumptionSaving models, so that they could be rewritten with a single source of truth about their model equations. These dynamics, in this PR, looked like this:
This is now essentially the same thing as what's included in the 'block' based consumption model, which was recently merged in: https://github.com/econ-ark/HARK/blob/master/HARK/models/consumer.py#L26-L44 Now that there's a compact format for model equations already in HARK, it would be a good idea to implement a solution to #1346 and put the equations onto the old models. |
Not getting what you mean by adding dynamics. In economists’ use of the
word these models already have dynamics. They are dynamic, stochastic,
optimization models.
I’m keen on making some basic structural decisions first.
Sent from Gmail Mobile
…On Sat, Jun 1, 2024 at 4:43 PM Sebastian Benthall ***@***.***> wrote:
I'd like to revisit this PR, which is now almost a year old, and perhaps
out of date with respect to recent HARK changes. But it was trying to
accomplish something that we are still trying to move towards.
The main point of this PR was to try attaching dynamics to the existing
ConsumptionSaving models, so that they could be rewritten with a single
source of truth about their model equations.
These dynamics, in this PR, looked like this:
IndShockConsumerType_dynamics = {
**PerfForesightConsumerType_dynamics,
'G' : lambda gamma, psi : gamma * psi,
'Rnrm' : lambda R, G : R / G,
'bNrm' : lambda Rnrm, aNrm : Rnrm * aNrm,
'mNrm' : lambda bNrm, theta : bNrm + theta,
'cNrm' : Control(['mNrm']),
'aNrm' : lambda mNrm, cNrm : mNrm - cNrm
}
This is now essentially the same thing as what's included in the 'block'
based consumption model, which was recently merged in:
https://github.com/econ-ark/HARK/blob/master/HARK/models/consumer.py#L26-L44
Now that there's a compact format for model equations already in HARK, it
would be a good idea to implement a solution to #1346
<#1346> and put the equations onto
the old models.
—
Reply to this email directly, view it on GitHub
<#1292 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKCK74KLV3MWV7UU3MWW7LZFIW53AVCNFSM6AAAAAAZWHG3XWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBTGU4DEMBXHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Just to clarify what I meant: I meant having a representation of the
equations governing how the variables change, which other parts of system
can use.
On Sun, Jun 2, 2024, 3:04 AM Christopher Llorracc Carroll <
***@***.***> wrote:
… Not getting what you mean by adding dynamics. In economists’ use of the
word these models already have dynamics. They are dynamic, stochastic,
optimization models.
I’m keen on making some basic structural decisions first.
Sent from Gmail Mobile
On Sat, Jun 1, 2024 at 4:43 PM Sebastian Benthall ***@***.***>
wrote:
> I'd like to revisit this PR, which is now almost a year old, and perhaps
> out of date with respect to recent HARK changes. But it was trying to
> accomplish something that we are still trying to move towards.
>
> The main point of this PR was to try attaching dynamics to the existing
> ConsumptionSaving models, so that they could be rewritten with a single
> source of truth about their model equations.
>
> These dynamics, in this PR, looked like this:
>
> IndShockConsumerType_dynamics = {
> **PerfForesightConsumerType_dynamics,
> 'G' : lambda gamma, psi : gamma * psi,
> 'Rnrm' : lambda R, G : R / G,
> 'bNrm' : lambda Rnrm, aNrm : Rnrm * aNrm,
> 'mNrm' : lambda bNrm, theta : bNrm + theta,
> 'cNrm' : Control(['mNrm']),
> 'aNrm' : lambda mNrm, cNrm : mNrm - cNrm
> }
>
> This is now essentially the same thing as what's included in the 'block'
> based consumption model, which was recently merged in:
>
>
>
https://github.com/econ-ark/HARK/blob/master/HARK/models/consumer.py#L26-L44
>
> Now that there's a compact format for model equations already in HARK,
it
> would be a good idea to implement a solution to #1346
> <#1346> and put the equations
onto
> the old models.
>
> —
> Reply to this email directly, view it on GitHub
> <#1292 (comment)>,
or
> unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/AAKCK74KLV3MWV7UU3MWW7LZFIW53AVCNFSM6AAAAAAZWHG3XWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBTGU4DEMBXHA>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
—
Reply to this email directly, view it on GitHub
<#1292 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAQZEHMEYOYVSPVHMJ3CO3ZFK7XRAVCNFSM6AAAAAAZWHG3XWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBTG4ZDMMJXGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
A reason to lay out the equations for more models is to validate that any
basic structural designs generalize beyond the portfolio consumption
problem.
As is, we risk spending months designing a general solution that works for
one specific problem.
…On Sun, Jun 2, 2024, 7:53 AM Sebastian Benthall ***@***.***> wrote:
Just to clarify what I meant: I meant having a representation of the
equations governing how the variables change, which other parts of system
can use.
On Sun, Jun 2, 2024, 3:04 AM Christopher Llorracc Carroll <
***@***.***> wrote:
> Not getting what you mean by adding dynamics. In economists’ use of the
> word these models already have dynamics. They are dynamic, stochastic,
> optimization models.
>
> I’m keen on making some basic structural decisions first.
>
> Sent from Gmail Mobile
>
>
> On Sat, Jun 1, 2024 at 4:43 PM Sebastian Benthall ***@***.***>
> wrote:
>
> > I'd like to revisit this PR, which is now almost a year old, and
> perhaps
> > out of date with respect to recent HARK changes. But it was trying to
> > accomplish something that we are still trying to move towards.
> >
> > The main point of this PR was to try attaching dynamics to the existing
> > ConsumptionSaving models, so that they could be rewritten with a single
> > source of truth about their model equations.
> >
> > These dynamics, in this PR, looked like this:
> >
> > IndShockConsumerType_dynamics = {
> > **PerfForesightConsumerType_dynamics,
> > 'G' : lambda gamma, psi : gamma * psi,
> > 'Rnrm' : lambda R, G : R / G,
> > 'bNrm' : lambda Rnrm, aNrm : Rnrm * aNrm,
> > 'mNrm' : lambda bNrm, theta : bNrm + theta,
> > 'cNrm' : Control(['mNrm']),
> > 'aNrm' : lambda mNrm, cNrm : mNrm - cNrm
> > }
> >
> > This is now essentially the same thing as what's included in the
> 'block'
> > based consumption model, which was recently merged in:
> >
> >
> >
> https://github.com/econ-ark/HARK/blob/master/HARK/models/consumer.py#L26-L44
> >
> > Now that there's a compact format for model equations already in HARK,
> it
> > would be a good idea to implement a solution to #1346
> > <#1346> and put the equations
> onto
> > the old models.
> >
> > —
> > Reply to this email directly, view it on GitHub
> > <#1292 (comment)>,
> or
> > unsubscribe
> > <
> https://github.com/notifications/unsubscribe-auth/AAKCK74KLV3MWV7UU3MWW7LZFIW53AVCNFSM6AAAAAAZWHG3XWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBTGU4DEMBXHA>
>
> > .
> > You are receiving this because you were mentioned.Message ID:
> > ***@***.***>
> >
>
> —
> Reply to this email directly, view it on GitHub
> <#1292 (comment)>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAAQZEHMEYOYVSPVHMJ3CO3ZFK7XRAVCNFSM6AAAAAAZWHG3XWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBTG4ZDMMJXGY>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
I don't think it will be months.
And once we have a satisfactory working example solution of two periods of
one particular problem, I am in complete agreement that we should take the
lessons learned and try adapting to another model. I'm thinking of the
EndogenousRetirement problem, because it is so different from ConsIndShk.
On Sun, Jun 2, 2024 at 10:14 AM Sebastian Benthall ***@***.***>
wrote:
… A reason to lay out the equations for more models is to validate that any
basic structural designs generalize beyond the portfolio consumption
problem.
As is, we risk spending months designing a general solution that works for
one specific problem.
On Sun, Jun 2, 2024, 7:53 AM Sebastian Benthall ***@***.***> wrote:
> Just to clarify what I meant: I meant having a representation of the
> equations governing how the variables change, which other parts of
system
> can use.
>
> On Sun, Jun 2, 2024, 3:04 AM Christopher Llorracc Carroll <
> ***@***.***> wrote:
>
>> Not getting what you mean by adding dynamics. In economists’ use of the
>> word these models already have dynamics. They are dynamic, stochastic,
>> optimization models.
>>
>> I’m keen on making some basic structural decisions first.
>>
>> Sent from Gmail Mobile
>>
>>
>> On Sat, Jun 1, 2024 at 4:43 PM Sebastian Benthall ***@***.***>
>> wrote:
>>
>> > I'd like to revisit this PR, which is now almost a year old, and
>> perhaps
>> > out of date with respect to recent HARK changes. But it was trying to
>> > accomplish something that we are still trying to move towards.
>> >
>> > The main point of this PR was to try attaching dynamics to the
existing
>> > ConsumptionSaving models, so that they could be rewritten with a
single
>> > source of truth about their model equations.
>> >
>> > These dynamics, in this PR, looked like this:
>> >
>> > IndShockConsumerType_dynamics = {
>> > **PerfForesightConsumerType_dynamics,
>> > 'G' : lambda gamma, psi : gamma * psi,
>> > 'Rnrm' : lambda R, G : R / G,
>> > 'bNrm' : lambda Rnrm, aNrm : Rnrm * aNrm,
>> > 'mNrm' : lambda bNrm, theta : bNrm + theta,
>> > 'cNrm' : Control(['mNrm']),
>> > 'aNrm' : lambda mNrm, cNrm : mNrm - cNrm
>> > }
>> >
>> > This is now essentially the same thing as what's included in the
>> 'block'
>> > based consumption model, which was recently merged in:
>> >
>> >
>> >
>>
https://github.com/econ-ark/HARK/blob/master/HARK/models/consumer.py#L26-L44
>> >
>> > Now that there's a compact format for model equations already in
HARK,
>> it
>> > would be a good idea to implement a solution to #1346
>> > <#1346> and put the equations
>> onto
>> > the old models.
>> >
>> > —
>> > Reply to this email directly, view it on GitHub
>> > <#1292 (comment)>,
>> or
>> > unsubscribe
>> > <
>>
https://github.com/notifications/unsubscribe-auth/AAKCK74KLV3MWV7UU3MWW7LZFIW53AVCNFSM6AAAAAAZWHG3XWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBTGU4DEMBXHA>
>>
>> > .
>> > You are receiving this because you were mentioned.Message ID:
>> > ***@***.***>
>> >
>>
>> —
>> Reply to this email directly, view it on GitHub
>> <#1292 (comment)>,
or
>> unsubscribe
>> <
https://github.com/notifications/unsubscribe-auth/AAAQZEHMEYOYVSPVHMJ3CO3ZFK7XRAVCNFSM6AAAAAAZWHG3XWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBTG4ZDMMJXGY>
>> .
>> You are receiving this because you were mentioned.Message ID:
>> ***@***.***>
>>
>
—
Reply to this email directly, view it on GitHub
<#1292 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKCK77ENUQ2BBA5UK7NRE3ZFMSCXAVCNFSM6AAAAAAZWHG3XWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBTHA3TAOBTG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
- Chris Carroll
|
This PR is aimed at several interrelated issues, including: #479 #857 and #889
The idea is to make the definition of the mathematical relationships between variables, 'equations' logically separate from the operational code that checks conditions, solves problems, and runs simulations.
Currently, the mathematical aspects of the model are blended in with the operational code. We've been trying to get away from this for some time.
This PR has one solution and a partial demonstration of how it would work. The Model class takes an
equations
argument, which is a dictionary. The equations are then used as appropriate. I've demonstrated how this refactoring could work for the assignments ofthorn
andaNrm
.This is just one way to do it. Here are two considerations:
resolve('thorn')
would:thorn
parameters
dictionary as inputthorn
assigned to the value).