Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
61 changes: 51 additions & 10 deletions HARK/ConsumptionSaving/ConsAggIndMarkovModel.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,37 @@ def _cond_mrkv_is_general(CondMrkvArrays):
)


def _cell_seed(base_entropy, macro_prev, macro_next, micro_prev):
"""Seed for one transition cell, addressed by content, not by call order.

The seed a cell gets must depend only on which cell it is, never on how
many other cells happened to be occupied first. Drawing it from
``self.RNG`` inside the loop ties it to position: the number and order of
non-empty cells depends on the realized macro path, so a scenario that
perturbs a few agents shifts every later cell's seed and redraws micro
states for agents it never touched. Measured at 15.5% of agents in one
such comparison.

That defeats the guarantee :meth:`MarkovProcess.draw` documents and goes
to some trouble to provide -- it spawns one sub-RNG per source state from
a single parent draw, precisely so an untouched row keeps its permutation
when a policy change alters a different row. The isolation was being
undone one level up.

Hoisting the draw above the loop is not sufficient and is the tempting
wrong fix: drawing one seed per *occupied* cell still indexes by position.
The seed has to come from the cell's identity, which is what this does.
``MarkovProcess.draw`` gets the same property by spawning over every
source state rather than every populated one.
"""
# macro_prev is None in the simple format, which conditions only on the
# destination. 0 stands for "not applicable" and the real indices shift
# up by one so they cannot collide with it.
mp = 0 if macro_prev is None else int(macro_prev) + 1
entropy = [base_entropy, mp, int(macro_next) + 1, int(micro_prev) + 1]
return int(np.random.SeedSequence(entropy).generate_state(1, dtype=np.uint32)[0])


def _zero_transition_msg(macro_prev, macro_next, micro_prev, n_agents):
"""Message for a macro transition that carries agents but no probability.

Expand Down Expand Up @@ -332,16 +363,19 @@ def _micro_transition_cells(self, general, macro_prev, macro_next, micro_prev, N
the destination; that is the value ``_zero_transition_msg`` expects
for the cell label it cannot report.

**Iteration order is load-bearing.** Every non-empty cell consumes
one draw from ``self.RNG`` for its ``MarkovProcess`` seed, so
reordering these changes every simulated value for a fixed seed.
The order here reproduces the two format-specific loops it replaced:
lexicographic in ``(macro_prev, macro_next)`` for the general format
(which is what ``np.unique(..., axis=0)`` returns), ascending in
Iteration order does **not** affect the draws. Each cell's seed comes
from :func:`_cell_seed`, which derives it from the cell's own
``(macro_prev, macro_next, micro_prev)`` identity, so a cell gets the
same seed no matter what else is or is not occupied. An earlier
version consumed one ``self.RNG`` draw per non-empty cell, which made
this order load-bearing and defeated common random numbers; see
``get_micro_markov_states``.

The order below still reproduces the two format-specific loops it
replaced: lexicographic in ``(macro_prev, macro_next)`` for the
general format (what ``np.unique(..., axis=0)`` returns), ascending in
``macro_next`` for the simple one, and ascending in ``micro_prev``
within each. Empty cells are yielded and skipped by the caller
rather than filtered here, which keeps the skip and the RNG draw in
one place.
within each.
"""
if general:
pairs = np.unique(np.column_stack([macro_prev, macro_next]), axis=0)
Expand Down Expand Up @@ -422,6 +456,11 @@ def get_micro_markov_states(self):
# _zero_transition_msg had already drifted apart in shape, which
# is what a validation branch looks like just before one copy
# stops matching the other.
# One draw for the whole call, then a per-cell seed derived from
# the cell's identity. See _cell_seed for why the draw cannot go
# inside the loop, and why hoisting it alone would not be enough.
base_entropy = int(self.RNG.integers(0, 2**63 - 1))

cells = self._micro_transition_cells(
general, macro_prev, macro_next, micro_prev, N
)
Expand All @@ -431,7 +470,9 @@ def get_micro_markov_states(self):
continue
if cond[mi, :].sum() <= 0.0:
raise ValueError(_zero_transition_msg(mp_i, mn_i, mi, n))
mp_proc = MarkovProcess(cond, seed=int(self.RNG.integers(0, 2**31 - 1)))
mp_proc = MarkovProcess(
cond, seed=_cell_seed(base_entropy, mp_i, mn_i, mi)
)
idx = np.flatnonzero(mask)
sort_key = np.asarray(pLvl_prev)[idx] if balanced else None
new_micro[idx] = mp_proc.draw(
Expand Down
1 change: 1 addition & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ Release Date: TBD
- Declares `init_shuffle` in `PerfForesightConsumerType_simulation_defaults` and `init_indshk_markov`, where its readers live; it was declared only on `IndShockConsumerType` and worked through a `getattr` fallback.
- Restores ruff's default file discovery (`extend-include` rather than `include`), so `ruff check <dir>` no longer reports "All checks passed" after inspecting zero Python files.
- `AgentType.get_states` now raises when `transition()` returns more values than there are states. States are assigned by position, so the loop silently dropped the tail. A return *shorter* than the state list stays legal, because it is deliberate: `GenIncProcessConsumerType` declares five states and returns three, writing the rest by name in `get_poststates`. The comment there now also records that reordering `state_vars` silently reassigns every value, which is how three models came to declare an `aNrm` that nothing wrote.
- **Changes simulated values under `markov_shuffle=True`.** `AggIndMrkvConsumerType.get_micro_markov_states` drew each transition cell's `MarkovProcess` seed from `self.RNG` *inside* a loop whose length and order depend on which cells are occupied, so a cell's seed was set by its position among populated cells rather than by which cell it is. That defeats the common-random-numbers property `MarkovProcess.draw` spawns one sub-RNG per source state to provide: a counterfactual that perturbs a few agents shifted every later cell's seed and redrew micro states for agents it never touched. Measured on a 400-agent fixture, adding 100 agents in a previously empty macro pair changed the micro state of 140 agents in untouched cells; a separate measurement put it at 15.5%. Seeds now come from the cell's own `(macro_prev, macro_next, micro_prev)` identity. Hoisting the draw above the loop is not a fix and the regression test rejects it (150 of 400 under that variant), because drawing one seed per *occupied* cell still indexes by position. Quota-exact transition counts are unchanged, being seed-invariant.
- `MarkovProcess.draw(shuffle=True)` now warns when a source state has too few agents for deterministic counts and falls back to iid. The exact transition counts the shuffled path advertises were silently withdrawn for those agents while the rest of the population kept them; both normalization mixins already warn on their analogous skips. Aggregated into one warning naming the affected source states, rather than one per state per period.
- `AgentType._sim_period_prologue` now blanks each period's ndarray states with `nan` instead of `np.empty`. A state that no later step writes previously held whatever was in the freed buffer, which in practice is usually the previous period's values, so the gap read as plausible data rather than as a defect; it now surfaces as `nan` rather than as plausible numbers. The same change in `AgentSimulator`'s newborn path (`HARK.simulator`) replaces an `np.empty` under a comment promising to "clear" the variable; both now route through one type-dispatching blank helper, which was already `nan`-filling elsewhere in that file. [#1809](https://github.com/econ-ark/HARK/issues/1809)
- Fixes three models reporting an uninitialized `aNrm`: `GenIncProcessConsumerType`, `MedShockConsumerType`, and `MedExtMargConsumerType`. The variable is declared in `state_vars` but these models work in levels, so nothing in `transition` writes it and only `sim_birth` ever touched it; every continuing agent carried whatever the per-period blanking left behind. On a 100-agent, 10-period `GenIncProcess` run, 998 of 1000 tracked cells disagreed with `aLvl / pLvl`, taking values like `3.96e-319` -- subnormals, i.e. freed memory. `MedShockConsumerType` was worse: all 1600 cells of a 200-agent, 8-period run. The definition now lives in one place, `GenIncProcessConsumerType.set_aNrm_from_levels`, which the two `get_poststates` overrides call, so a further override cannot silently reopen it. Found because the `nan` blanking above made the second and third instances visible.
Expand Down
47 changes: 47 additions & 0 deletions tests/ConsumptionSaving/test_ConsAggIndMarkovModel.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,53 @@ def test_general_format_per_cell_counts(self):
),
)

def test_untouched_cells_keep_their_draws_when_other_cells_appear(self):
"""Common random numbers: a cell's draw must not depend on its neighbours.

This is the property ``MarkovProcess.draw`` spawns one sub-RNG per
source state to provide, so that a counterfactual altering one
transition row leaves the other rows' permutations alone. It was
being undone here: each cell's seed came from ``self.RNG`` *inside*
the loop, so the number and order of occupied cells fixed every later
cell's seed.

The perturbation has to change **which** ``(macro_prev, macro_next)``
pairs are realized. Varying ``AgentCount`` alone changes cell
populations but not the cell list, so a fix that merely hoists the
draw above the loop passes that version of this test while leaving
the defect in place.
"""
cond, macro_prev, macro_next, micro_prev = self._two_pair_setup()

baseline = self._make_agent(cond, macro_prev, macro_next, micro_prev)
baseline.get_micro_markov_states()
before = baseline.MicroMrkvNow.copy()

# The same 400 agents, plus 100 in the (0, 0) pair, which was empty.
# That inserts occupied cells ahead of the ones under test.
extra = 100
macro_prev_b = np.concatenate([macro_prev, np.zeros(extra, dtype=int)])
macro_next_b = np.concatenate([macro_next, np.zeros(extra, dtype=int)])
micro_prev_b = np.concatenate([micro_prev, np.zeros(extra, dtype=int)])

perturbed = self._make_agent(cond, macro_prev_b, macro_next_b, micro_prev_b)
perturbed.get_micro_markov_states()
after = perturbed.MicroMrkvNow[: macro_prev.size]

n_diff = int(np.sum(before != after))
self.assertEqual(
n_diff,
0,
msg=(
f"{n_diff} of {before.size} agents in untouched "
f"(macro_prev, macro_next) cells changed micro state when an "
f"unrelated cell became occupied. Their transition rows and "
f"agent sets are identical across the two runs, so the draws "
f"must be too; a nonzero count means cell seeds still depend "
f"on position in the iteration rather than on cell identity."
),
)

def test_general_format_all_agents_assigned(self):
"""Every agent leaves with a micro state in range.

Expand Down
Loading