What is update? How many terminal solutions?

If we are lucky, it's calling the same update_terminal_solution 3 times. I don't understand this enough but I think I have been in situations where, due to poor inheritance practices, the update_teriminal_solution that I expected is not the one being run, or at least not the last one being run.

This is a big issue that sometimes makes debugging solutions harder than it needs to be.

alanlujan91 commented 1 month ago

I think the solution is to remove the following lines

https://github.com/econ-ark/HARK/blob/3ee397993a1db76bbc9ae24d3196dc31ccfe4874/HARK/ConsumptionSaving/ConsPortfolioModel.py#L174-L178

but this might not be the only model that does something like this

mnwhite commented 1 month ago

The good news is that as long as all of the classes are doing self.update_terminal_solution(), they will call the right one. The bad news is that we're doing redundant work. For the record, pre_solve is probably the right place for it to be called (but update is often called by pre_solve, so...?).

On Fri, Apr 5, 2024 at 7:44 PM Alan Lujan @.***> wrote:

I think the solution is to remove the following lines

https://github.com/econ-ark/HARK/blob/3ee397993a1db76bbc9ae24d3196dc31ccfe4874/HARK/ConsumptionSaving/ConsPortfolioModel.py#L174-L178

but this might not be the only model that does something like this

— Reply to this email directly, view it on GitHub https://github.com/econ-ark/HARK/issues/1405#issuecomment-2040770855, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADKRAFLGGIUYMNXBQBQ37C3Y34ZONAVCNFSM6AAAAABFZ5UZJ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBQG43TAOBVGU . You are receiving this because you were assigned.Message ID: @.***>

alanlujan91 commented 1 month ago

Why don't we use super?

mnwhite commented 1 month ago

Because I don't think we want to use the parent class version at any point. I could be wrong, however.

On Fri, Apr 5, 2024 at 8:04 PM Alan Lujan @.***> wrote:

Why don't we use super?

— Reply to this email directly, view it on GitHub https://github.com/econ-ark/HARK/issues/1405#issuecomment-2040800982, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADKRAFJ6UHMA6SYQV64W66TY343ZLAVCNFSM6AAAAABFZ5UZJ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBQHAYDAOJYGI . You are receiving this because you were assigned.Message ID: @.***>

alanlujan91 commented 1 month ago

No I mean in general, super at instantiation

Mv77 commented 1 month ago

Yeah, this is a problem. I think some agent types run the "create income process" from ConsIndShock as much as 3 times each for a single solve. Which can take longer than the solve itself...

Mv77 commented 1 month ago

Maybe a policy for this would be that every agent type should define (not simply inherit) its full update, pre_solve, and post_solve methods? You'd be free to call super.update_whatever(...) in them, but at least you would be doing it consciously?

mnwhite commented 1 month ago

Yes, something like that will likely be part of the solution. This is the kind of problem that will be fixed by the parameters overhaul I'm beginning work on now, to make constructed inputs more systematic and structured. The terminal solution is a special constructed input, in that it's required.

On Sat, Apr 6, 2024, 10:01 AM Mateo Velásquez-Giraldo < @.***> wrote:

Maybe a policy for this would be that every agent type should define (not simply inherit) its full update, pre_solve, and post_solve methods? You'd be free to call super.update_whatever(...) in them, but at least you would be doing it consciously?

— Reply to this email directly, view it on GitHub https://github.com/econ-ark/HARK/issues/1405#issuecomment-2041097120, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADKRAFMQLIOVAF6BEXUY6BDY3754DAVCNFSM6AAAAABFZ5UZJ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBRGA4TOMJSGA . You are receiving this because you were assigned.Message ID: @.***>

econ-ark / HARK

What is update? How many terminal solutions? #1405