Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 38 additions & 4 deletions HARK/ConsumptionSaving/ConsPortfolioModel.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from HARK import AgentType, NullFunc, make_one_period_oo_solver
from HARK.ConsumptionSaving.ConsIndShockModel import (
ConsIndShockSolver,
IndShockConsumerType,
init_idiosyncratic_shocks,
utility,
Expand Down Expand Up @@ -238,6 +239,11 @@ def update_solution_terminal(self):
dvdsFuncFxd=dvdsFuncFxd_terminal,
)

self.solution_terminal.mNrmMin = 0.0
self.solution_terminal.hNrm = 0.0
self.solution_terminal.MPCmin = 1.0
self.solution_terminal.MPCmax = 1.0

def initialize_sim(self):
"""
Initialize the state of simulation attributes. Simply calls the same method
Expand Down Expand Up @@ -330,7 +336,7 @@ def __init__(self, verbose=False, quiet=False, **kwds):
self.solve_one_period = make_one_period_oo_solver(ConsSequentialPortfolioSolver)


class ConsPortfolioSolver(MetricObject):
class ConsPortfolioSolver(ConsIndShockSolver):
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class does not call ConsIndShockSetup.init during initialization. (ConsPortfolioSolver.init may be missing a call to a base class init)

Copilot uses AI. Check for mistakes.
"""
Define an object-oriented one period solver.
Solve the one period problem for a portfolio-choice consumer.
Expand Down Expand Up @@ -433,6 +439,8 @@ def __init__(
self.ShareLimit = ShareLimit
self.IndepDstnBool = IndepDstnBool

self.CubicBool = False # This solver doesn't use cubic interpolation

# Make sure the individual is liquidity constrained. Allowing a consumer to
# borrow *and* invest in an asset with unbounded (negative) returns is a bad mix.
if BoroCnstArt != 0.0:
Expand All @@ -447,6 +455,9 @@ def __init__(

self.def_utility_funcs()

self.solution_next.vFunc = self.solution_next.vFuncAdj # this is a hack
self.solution_next.vPfunc = self.solution_next.vPfuncAdj # this is a hack
Comment on lines +458 to +459
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment "this is a hack" is not sufficiently descriptive. Consider expanding it to explain why this assignment is necessary, something like: "Map portfolio-specific vFuncAdj and vPfuncAdj to the generic vFunc and vPfunc names expected by the parent class's set_and_update_values method."

Suggested change
self.solution_next.vFunc = self.solution_next.vFuncAdj # this is a hack
self.solution_next.vPfunc = self.solution_next.vPfuncAdj # this is a hack
self.solution_next.vFunc = self.solution_next.vFuncAdj # Map portfolio-adjusted value function to generic vFunc name expected by the parent class's set_and_update_values method.
self.solution_next.vPfunc = self.solution_next.vPfuncAdj # Map portfolio-adjusted marginal value function to generic vPfunc name expected by the parent class's set_and_update_values method.

Copilot uses AI. Check for mistakes.

def def_utility_funcs(self):
"""
Define temporary functions for utility and its derivative and inverse
Expand All @@ -458,12 +469,16 @@ def def_utility_funcs(self):
self.uinv = lambda x: utility_inv(x, self.CRRA)
self.uinvP = lambda x: utility_invP(x, self.CRRA)

def set_and_update_values(self):
def set_and_update_values(self, solution_next, IncShkDstn, LivPrb, DiscFac):
"""
Unpacks some of the inputs (and calculates simple objects based on them),
storing the results in self for use by other methods.
"""

super().set_and_update_values(
self.solution_next, self.IncShkDstn, self.LivPrb, self.DiscFac
)
Comment on lines +478 to +480
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method signature declares parameters (solution_next, IncShkDstn, LivPrb, DiscFac) but then ignores them and uses self.solution_next, self.IncShkDstn, self.LivPrb, self.DiscFac instead when calling super(). This is confusing and inconsistent. Either remove the unused parameters from the signature, or use the parameters instead of self.* attributes.

Suggested change
super().set_and_update_values(
self.solution_next, self.IncShkDstn, self.LivPrb, self.DiscFac
)
super().set_and_update_values(solution_next, IncShkDstn, LivPrb, DiscFac)

Copilot uses AI. Check for mistakes.

# Unpack next period's solution
self.vPfuncAdj_next = self.solution_next.vPfuncAdj
self.dvdmFuncFxd_next = self.solution_next.dvdmFuncFxd
Expand All @@ -482,7 +497,10 @@ def prepare_to_solve(self):
Perform preparatory work.
"""

self.set_and_update_values()
self.set_and_update_values(
self.solution_next, self.IncShkDstn, self.LivPrb, self.DiscFac
)
self.def_BoroCnst(self.BoroCnstArt)

def prepare_to_calc_EndOfPrdvP(self):
"""
Expand Down Expand Up @@ -925,6 +943,10 @@ def solve(self):

self.make_porfolio_solution()

self.solution = self.add_MPC_and_human_wealth(self.solution)
self.solution.mNrmMin = self.mNrmMinNow
self.solution = self.add_stable_points(self.solution)
Comment on lines +946 to +948
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No tests have been added to verify the stable points functionality (mNrmStE and mNrmTrg) for the portfolio model. Given that this is a new feature and there are existing tests for stable points in test_IndShockConsumerType.py and test_PerfForesightConsumerType.py, tests should be added to verify that the stable points are correctly calculated for portfolio consumers.

Copilot uses AI. Check for mistakes.
Comment on lines +946 to +948
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The add_stable_points method inherited from ConsIndShockSolver expects the solution object to have a cFunc attribute. However, PortfolioSolution uses cFuncAdj and cFuncFxd instead of cFunc. When add_stable_points calls add_mNrmStE or add_mNrmTrg, these methods will attempt to evaluate solution.cFunc(m), which will fail with an AttributeError. To fix this, either add a cFunc attribute to PortfolioSolution that maps to the appropriate function (likely cFuncAdj), or override add_stable_points in ConsPortfolioSolver to handle the portfolio-specific solution structure.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The calculation of stable points using the ConsIndShockSolver methodology may not be conceptually appropriate for the portfolio model. The stable points (mNrmStE and mNrmTrg) in ConsIndShockModel represent target wealth levels assuming a single consumption function. However, in the portfolio model, consumption depends on both market resources and the risky portfolio share, with different functions for when the agent can adjust (cFuncAdj) versus when they cannot (cFuncFxd). The stable point calculations should likely account for the portfolio allocation decision and adjustment probability, not just use the adjustable consumption function. Consider developing a portfolio-specific stable points calculation that properly incorporates the two-dimensional nature of the problem.

Suggested change
self.solution = self.add_stable_points(self.solution)
# Stable points from ConsIndShockSolver are not conceptually appropriate
# for the portfolio model (which is two-dimensional and has cFuncAdj/cFuncFxd).
# Explicitly mark them as unavailable instead of using the 1D logic.
self.solution.mNrmStE = None
self.solution.mNrmTrg = None

Copilot uses AI. Check for mistakes.

return self.solution


Expand Down Expand Up @@ -995,6 +1017,10 @@ def solve(self):

self.make_porfolio_solution()

self.solution = self.add_MPC_and_human_wealth(self.solution)
self.solution.mNrmMin = self.mNrmMinNow
self.solution = self.add_stable_points(self.solution)
Comment on lines +1020 to +1022
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The add_stable_points method inherited from ConsIndShockSolver expects the solution object to have a cFunc attribute. However, PortfolioSolution uses cFuncAdj and cFuncFxd instead of cFunc. When add_stable_points calls add_mNrmStE or add_mNrmTrg, these methods will attempt to evaluate solution.cFunc(m), which will fail with an AttributeError. To fix this, either add a cFunc attribute to PortfolioSolution that maps to the appropriate function (likely cFuncAdj), or override add_stable_points in ConsPortfolioSolver to handle the portfolio-specific solution structure.

Copilot uses AI. Check for mistakes.

return self.solution


Expand All @@ -1007,12 +1033,16 @@ class ConsPortfolioJointDistSolver(ConsPortfolioDiscreteSolver, ConsPortfolioSol
discrete.
"""

def set_and_update_values(self):
def set_and_update_values(self, solution_next, IncShkDstn, LivPrb, DiscFac):
"""
Unpacks some of the inputs (and calculates simple objects based on them),
storing the results in self for use by other methods.
"""

super().set_and_update_values(
self.solution_next, self.IncShkDstn, self.LivPrb, self.DiscFac
)
Comment on lines +1042 to +1044
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method signature declares parameters (solution_next, IncShkDstn, LivPrb, DiscFac) but then ignores them and uses self.solution_next, self.IncShkDstn, self.LivPrb, self.DiscFac instead when calling super(). This is confusing and inconsistent. Either remove the unused parameters from the signature, or use the parameters instead of self.* attributes.

Suggested change
super().set_and_update_values(
self.solution_next, self.IncShkDstn, self.LivPrb, self.DiscFac
)
super().set_and_update_values(solution_next, IncShkDstn, LivPrb, DiscFac)

Copilot uses AI. Check for mistakes.

# Unpack next period's solution
self.vPfuncAdj_next = self.solution_next.vPfuncAdj
self.dvdmFuncFxd_next = self.solution_next.dvdmFuncFxd
Expand Down Expand Up @@ -1220,6 +1250,10 @@ def solve(self):

self.make_porfolio_solution()

Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The add_stable_points method inherited from ConsIndShockSolver expects the solution object to have a cFunc attribute. However, PortfolioSolution uses cFuncAdj and cFuncFxd instead of cFunc. When add_stable_points calls add_mNrmStE or add_mNrmTrg, these methods will attempt to evaluate solution.cFunc(m), which will fail with an AttributeError. To fix this, either add a cFunc attribute to PortfolioSolution that maps to the appropriate function (likely cFuncAdj), or override add_stable_points in ConsPortfolioSolver to handle the portfolio-specific solution structure.

Suggested change
# Ensure the solution object has a cFunc attribute for methods inherited
# from ConsIndShockSolver that expect solution.cFunc(m).
if not hasattr(self.solution, "cFunc"):
if hasattr(self.solution, "cFuncAdj"):
self.solution.cFunc = self.solution.cFuncAdj
elif hasattr(self.solution, "cFuncFxd"):
self.solution.cFunc = self.solution.cFuncFxd

Copilot uses AI. Check for mistakes.
self.solution = self.add_MPC_and_human_wealth(self.solution)
self.solution.mNrmMin = self.mNrmMinNow
self.solution = self.add_stable_points(self.solution)

return self.solution


Expand Down
2 changes: 1 addition & 1 deletion HARK/ConsumptionSaving/ConsRiskyAssetModel.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from scipy.optimize import minimize_scalar, root_scalar

from HARK import make_one_period_oo_solver
from HARK.ConsumptionSaving.ConsIndShockModel import ( # PortfolioConsumerType inherits from it; Baseline dictionary to build on
from HARK.ConsumptionSaving.ConsIndShockModel import (
ConsIndShockSolver,
ConsumerSolution,
IndShockConsumerType,
Expand Down
Loading