Skip to content

[WIP]: normalize_shocks and normalize_levels allow user to impose true propositions on sims #1094

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.

Sign up for GitHub

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

Open
llorracc wants to merge 9 commits into
base: main
Choose a base branch
from
+38 −5
Open

[WIP]: normalize_shocks and normalize_levels allow user to impose true propositions on sims #1094

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
963f0c0
Clean up and rationalize
llorracc Dec 23, 2021
362d3de
Restore update_income_process() in pre_solve
llorracc Jan 4, 2022
387e57b
Make PR easier to read
llorracc Jan 4, 2022
6f35645
Further improvement in clarity
llorracc Jan 4, 2022
c2fe146
Even simpler
llorracc Jan 4, 2022
0b11590
Final clarity tweak
llorracc Jan 4, 2022
b4c668a
Make sure normalize booleans have values
llorracc Jan 4, 2022
41f944d
Move test for hasattribute normalizations to simulation code
llorracc Jan 4, 2022
8f0057e
Remove unneeded comments
llorracc Jan 5, 2022
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
43 changes: 38 additions & 5 deletions HARK/ConsumptionSaving/ConsIndShockModel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1794,6 +1794,20 @@ def transition(self):

# Calculate new states: normalized market resources and permanent income level
pLvlNow = pLvlPrev*self.shocks['PermShk'] # Updated permanent income level
# Asymptotically it can't hurt to impose true restrictions
# (at least if the GICRaw holds)
pLvlNowMean = 1.0
if not hasattr(self, "normalize_shocks"):
self.normalize_shocks = False

Comment on lines +1800 to +1802
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 normalize_shocks attribute is checked and set in the transition() method (lines 1800-1801) but is never used anywhere in this method. This appears to be dead code. The normalize_shocks parameter is only relevant in the get_shocks() method of IndShockConsumerType, not in the transition() method of PerfForesightConsumerType. Consider removing these lines.

Suggested change
if not hasattr(self, "normalize_shocks"):
self.normalize_shocks = False

Copilot uses AI. Check for mistakes.
if not hasattr(self, "normalize_levels"):
self.normalize_levels = False

Comment on lines +1802 to +1805
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.

Inconsistent spacing in comment. There's trailing whitespace after the inline comment, which should be removed for consistency with Python style guidelines.

Suggested change
if not hasattr(self, "normalize_levels"):
self.normalize_levels = False
if not hasattr(self, "normalize_levels"):
self.normalize_levels = False

Copilot uses AI. Check for mistakes.
if self.normalize_levels == True:
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.

Using '== True' is redundant in Python. The idiomatic way to check boolean values is to use 'if self.normalize_levels:' instead of 'if self.normalize_levels == True:'.

Suggested change
if self.normalize_levels == True:
if self.normalize_levels:

Copilot uses AI. Check for mistakes.
Comment on lines +1800 to +1806
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.

Using hasattr checks in the transition method is problematic. These checks will only be executed once per simulation step, and if the attributes don't exist initially, they'll be set to False dynamically. This approach is fragile and unconventional. Instead, the parameters should be initialized properly in the agent's initialization or retrieved from the parameter dictionary. Consider using getattr with a default value instead, such as: self.normalize_shocks = getattr(self, 'normalize_shocks', False)

Suggested change
if not hasattr(self, "normalize_shocks"):
self.normalize_shocks = False
if not hasattr(self, "normalize_levels"):
self.normalize_levels = False
if self.normalize_levels == True:
# Use getattr with a default instead of dynamically creating attributes here
if getattr(self, "normalize_levels", False):

Copilot uses AI. Check for mistakes.
pLvlNowMean = np.mean(pLvlNow)
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.

Potential division by zero issue. If all agents have pLvlNow equal to zero (or very close to zero), np.mean(pLvlNow) could be zero or near-zero, leading to a division by zero or numerical instability. This should be guarded against, perhaps by only normalizing when the mean is above a certain threshold.

Suggested change
pLvlNowMean = np.mean(pLvlNow)
pLvlNowMean = np.mean(pLvlNow)
# Guard against zero, near-zero, or non-finite means to avoid division by zero
if (not np.isfinite(pLvlNowMean)) or (abs(pLvlNowMean) < 1e-8):
pLvlNowMean = 1.0

Copilot uses AI. Check for mistakes.

pLvlNow = pLvlNow / pLvlNowMean # Divide by 1.0 if normalize_levels=False
Comment on lines +1800 to +1809
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 normalize_levels parameter is used in the PerfForesightConsumerType.transition() method but is only added to init_idiosyncratic_shocks, not to init_perfect_foresight. This means that PerfForesightConsumerType agents won't have this parameter by default, and the hasattr check will always set it to False. If normalize_levels is intended to be used by PerfForesightConsumerType, it should also be added to init_perfect_foresight with a default value of False.

Copilot uses AI. Check for mistakes.

# Updated aggregate permanent productivity level
PlvlAggNow = self.state_prev['PlvlAgg']*self.PermShkAggNow
# "Effective" interest factor on normalized assets
Expand Down Expand Up @@ -2037,6 +2051,8 @@ def check_conditions(self, verbose=None):
"vFuncBool": False, # Whether to calculate the value function during solution
"CubicBool": False, # Use cubic spline interpolation when True, linear interpolation when False
"neutral_measure": False, # Use permanent income neutral measure (see Harmenberg 2021) during simulations when True.
"normalize_shocks": False, # In sims, normalize mean of collection of shocks to population mean
"normalize_levels": False, # In sims, normalize mean of a level variable (like permanent income) to the population mean
Comment on lines +2054 to +2055
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 new normalize_shocks and normalize_levels parameters lack test coverage. Given that this repository has comprehensive test suites for ConsIndShockModel (as seen in test_IndShockConsumerType.py), these new features should be tested to verify: (1) that normalization works correctly when enabled, (2) that simulations produce expected results with and without normalization, and (3) that edge cases (like near-zero means) are handled properly.

Copilot uses AI. Check for mistakes.
Comment on lines +2054 to +2055
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 new parameters normalize_shocks and normalize_levels are added to the init_idiosyncratic_shocks dictionary but are not documented in the class docstring. The IndShockConsumerType class docstring (lines 2061-2072) should be updated to document these new parameters, explaining what they do and when they should be used.

Copilot uses AI. Check for mistakes.
}
)

Expand Down Expand Up @@ -2192,10 +2208,18 @@ def get_shocks(self):
# Get random draws of income shocks from the discrete distribution
IncShks = IncShkDstnNow.draw(N)

# In the limit, it cannot hurt to impose "true" restrictions,
# like the fact that the mean value of the shocks should be one
PermShkMeanNow, TranShkMeanNow = 1.0, 1.0 # Dividing by 1 changes nothing
if self.normalize_shocks == True:
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.

Using '== True' is redundant in Python. The idiomatic way to check boolean values is to use 'if self.normalize_shocks:' instead of 'if self.normalize_shocks == True:'.

Suggested change
if self.normalize_shocks == True:
if self.normalize_shocks:

Copilot uses AI. Check for mistakes.
PermShkMeanNow = np.mean(IncShks[0])
TranShkMeanNow = np.mean(IncShks[1])

PermShkNow[these] = (
IncShks[0, :] * PermGroFacNow
(IncShks[0, :] * PermGroFacNow
/ PermShkMeanNow) # Divide by 1.0 if normalize_shocks=False
) # permanent "shock" includes expected growth
Comment on lines 2218 to 2221
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 normalization logic may be mathematically incorrect. When normalize_shocks is True, the permanent shock is computed as (IncShks[0, :] * PermGroFacNow / PermShkMeanNow). However, PermShkMeanNow is calculated from the raw shock values (IncShks[0]), not from the values after multiplying by PermGroFacNow. This means the normalization is forcing the mean of the raw shocks to be 1, but the actual permanent shocks (which include PermGroFacNow) will have a mean of PermGroFacNow, not 1. If the intent is to normalize the final permanent shocks to have mean PermGroFacNow, then the calculation should be: PermShkMeanNow = np.mean(IncShks[0] * PermGroFacNow).

Copilot uses AI. Check for mistakes.
TranShkNow[these] = IncShks[1, :]
TranShkNow[these] = IncShks[1, :] / TranShkMeanNow
Comment on lines +2215 to +2222
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.

Potential division by zero issue. If the sampled shocks have a mean of exactly zero (or very close to zero), dividing by PermShkMeanNow or TranShkMeanNow will cause division by zero or numerical instability. This should be guarded against, perhaps by only normalizing when the mean is above a certain threshold.

Copilot uses AI. Check for mistakes.

# That procedure used the *last* period in the sequence for newborns, but that's not right
# Redraw shocks for newborns, using the *first* period in the sequence. Approximation.
Expand All @@ -2207,10 +2231,19 @@ def get_shocks(self):

# Get random draws of income shocks from the discrete distribution
EventDraws = IncShkDstnNow.draw_events(N)

# In the limit, it cannot hurt to impose "true" restrictions,
# like the fact that the mean value of the shocks should be one
PermShkMeanNow, TranShkMeanNow = 1.0, 1.0 # Dividing by 1 changes nothing
if self.normalize_shocks == True:
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.

Using '== True' is redundant in Python. The idiomatic way to check boolean values is to use 'if self.normalize_shocks:' instead of 'if self.normalize_shocks == True:'.

Copilot uses AI. Check for mistakes.
PermShkMeanNow = np.mean(IncShkDstnNow.X[0][EventDraws])
TranShkMeanNow = np.mean(IncShkDstnNow.X[1][EventDraws])

PermShkNow[these] = (
IncShkDstnNow.X[0][EventDraws] * PermGroFacNow
(IncShkDstnNow.X[0][EventDraws] * PermGroFacNow
/ PermShkMeanNow) # Divide by 1.0 if normalize_shocks=False
) # permanent "shock" includes expected growth
Comment on lines 2242 to 2245
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 normalization logic may be mathematically incorrect. When normalize_shocks is True, the permanent shock is computed as (IncShkDstnNow.X[0][EventDraws] * PermGroFacNow / PermShkMeanNow). However, PermShkMeanNow is calculated from the raw shock values (IncShkDstnNow.X[0][EventDraws]), not from the values after multiplying by PermGroFacNow. This means the normalization is forcing the mean of the raw shocks to be 1, but the actual permanent shocks (which include PermGroFacNow) will have a mean of PermGroFacNow, not 1. If the intent is to normalize the final permanent shocks to have mean PermGroFacNow, then the calculation should be: PermShkMeanNow = np.mean(IncShkDstnNow.X[0][EventDraws] * PermGroFacNow).

Copilot uses AI. Check for mistakes.
TranShkNow[these] = IncShkDstnNow.X[1][EventDraws]
TranShkNow[these] = IncShkDstnNow.X[1][EventDraws] / TranShkMeanNow
Comment on lines +2239 to +2246
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.

Potential division by zero issue. If the sampled shocks have a mean of exactly zero (or very close to zero), dividing by PermShkMeanNow or TranShkMeanNow will cause division by zero or numerical instability. This should be guarded against, perhaps by only normalizing when the mean is above a certain threshold.

Copilot uses AI. Check for mistakes.
# PermShkNow[newborn] = 1.0
TranShkNow[newborn] = 1.0

Expand Down Expand Up @@ -3067,4 +3100,4 @@ def construct_assets_grid(parameters):
init_cyclical['PermShkStd'] = [0.1, 0.1, 0.1, 0.1]
init_cyclical['TranShkStd'] = [0.1, 0.1, 0.1, 0.1]
init_cyclical['LivPrb'] = 4*[0.98]
init_cyclical['T_cycle'] = 4
init_cyclical['T_cycle'] = 4
Loading