Repository navigation
skip_failed_choices can record person IDs as household IDs during tour destination sampling #1137
Description
Problem
When skip_failed_choices=True, failed tour-destination sampling can record a person ID as a skipped household ID.
This was identified while reviewing #1130, but it is a pre-existing bug, not a regression introduced by that PR. The reproducer below fails on both:
- Base revision
cc1944cb4fdf56cdebdbf9e542873ac9ee2a4f95. - PR revision
2d6043a0e4b7e99300bb5adebecd329abda00fbf.
Cause and impact
The merge in run_destination_sample combines tours and persons that both contain household_id, producing household_id_x and household_id_y. When person_id is retained in the sampling columns, tracing.trace_id_for_chooser falls back to that column. logit.report_bad_choices then treats the returned person IDs as household IDs.
For person 1 in household 11, the sampler records skipped household 1, rather than 11. This can cause an unrelated household with ID 1 to be removed, and makes the skip bookkeeping incorrect. The additional bookkeeping in #1130 can add the correct household 11 afterward, but does not remove the erroneous entry (yielding {1, 11}).
The reproducer explicitly sets ComputeSettings(drop_unused_columns=False) to retain person_id. This is a conditional failure, not a claim that every default sampling configuration encounters it.
Reproducer
Save as repro_skipped_household_identity.py and run in an ActivitySim environment against either revision above:
PYTHONPATH=/path/to/activitysim-checkout python repro_skipped_household_identity.pyNo model data or mocks are required. The script recreates the tour/person merge used by the destination model and calls the real sampler; it does not run an entire model.
"""Reproduce person IDs being recorded as skipped household IDs.
Verified on cc1944c (PR #1130 base) and 2d6043a (PR head).
No model data or mocks are required.
"""
from pathlib import Path
from tempfile import TemporaryDirectory
import pandas as pd
from activitysim.core import interaction_sample, workflow
from activitysim.core.configuration.base import ComputeSettings
def reproduce():
"""Exercise the sampler with the tour/person merge used by destination choice."""
with TemporaryDirectory() as directory:
root = Path(directory)
(root / "configs").mkdir()
(root / "data").mkdir()
state = workflow.State().default_settings()
state.initialize_filesystem(working_dir=root, configs_dir=("configs",))
state.settings.skip_failed_choices = True
state.settings.sharrow = False
state.settings.use_explicit_error_terms = False
state.settings.check_for_variability = False
tours = pd.DataFrame(
{"household_id": [11, 12], "person_id": [1, 2]},
index=pd.Index([101, 102], name="tour_id"),
)
persons = pd.DataFrame(
{"household_id": [11, 12], "utility": [-1000.0, 0.0]},
index=pd.Index([1, 2], name="person_id"),
)
# Same merge as run_destination_sample: household_id becomes _x and _y.
choosers = tours.merge(
persons, left_on="person_id", right_index=True, how="left"
)
state.rng().add_channel("tour_id", tours)
state.rng().begin_step("reproduce")
interaction_sample.interaction_sample(
state,
choosers,
alternatives=pd.DataFrame(
{"unused": [0.0, 0.0]}, index=pd.Index([10, 20], name="zone_id")
),
spec=pd.DataFrame(
{"coefficient": [1.0]},
index=pd.Index(["utility"], name="Expression"),
),
sample_size=2,
alt_col_name="zone_id",
# Retain person_id, exposing the tracing fallback to that column.
compute_settings=ComputeSettings(drop_unused_columns=False),
)
skipped = {
int(household_id)
for ids in state.get("skipped_household_ids", {}).values()
for household_id in ids
}
print("Expected skipped households: [11]")
print("Actual skipped households:", sorted(skipped))
assert skipped == {11}, f"Incorrect skipped households: {sorted(skipped)}"
if __name__ == "__main__":
reproduce()Observed on both revisions:
Expected skipped households: [11]
Actual skipped households: [1]
AssertionError: Incorrect skipped households: [1]
Expected behavior / proposed acceptance criteria
- Preserve an unambiguous household ID through the merge and sampling/error-reporting path.
- Never insert a person ID into
skipped_household_idsmerely because household identity is unavailable. - Cover distinct person/household IDs, overlapping ID values across unrelated households, and retained versus pruned sampling columns.
- Verify that the failed chooser's actual household is recorded exactly once and unrelated households remain unaffected.
Activity