Deprecate SIMULATE_CHOOSER_COLUMNS and LOGSUM_CHOOSER_COLUMNS settings (#1094)

* Initial plan

* Deprecate SIMULATE_CHOOSER_COLUMNS and LOGSUM_CHOOSER_COLUMNS settings

* Drop unused columns in interaction_sample even when tracing

* Fix chooser column deprecation regressions

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Jeff Newman <jeff@driftless.xyz>

Author: Copilot

activitysim/abm/models/location_choice.py

@@ -242,14 +242,10 @@ def location_sample(
     chunk_tag,
     trace_label,
 ):
-    # FIXME - MEMORY HACK - only include columns actually used in spec
-    chooser_columns = model_settings.SIMULATE_CHOOSER_COLUMNS
-    # Drop this when PR #1017 is merged
-    if ("household_id" not in chooser_columns) and (
-        "household_id" in persons_merged.columns
-    ):
-        chooser_columns = chooser_columns + ["household_id"]
-    choosers = persons_merged[chooser_columns]
+    # The former column selection returned an independent frame.  Preserve that
+    # isolation so component preprocessors cannot leak annotations into the
+    # shared persons table or into later location-choice segments.
+    choosers = persons_merged.copy()
 
     # create wrapper with keys for this lookup - in this case there is a home_zone_id in the choosers
     # and a zone_id in the alternatives which get merged during interaction
@@ -441,17 +437,8 @@ def location_presample(
         HOME_TAZ in persons_merged
     )  # 'TAZ' should already be in persons_merged from land_use
 
-    # FIXME - MEMORY HACK - only include columns actually used in spec
-    # FIXME we don't actually require that land_use provide a TAZ crosswalk
-    # FIXME maybe we should add it for multi-zone (from maz_taz) if missing?
-    chooser_columns = model_settings.SIMULATE_CHOOSER_COLUMNS
-    chooser_columns = [HOME_TAZ if c == HOME_MAZ else c for c in chooser_columns]
-    # Drop this when PR #1017 is merged
-    if ("household_id" not in chooser_columns) and (
-        "household_id" in persons_merged.columns
-    ):
-        chooser_columns = chooser_columns + ["household_id"]
-    choosers = persons_merged[chooser_columns]
+    # Keep chooser annotations local to this model segment.
+    choosers = persons_merged.copy()
 
     # create wrapper with keys for this lookup - in this case there is a HOME_TAZ in the choosers
     # and a DEST_TAZ in the alternatives which get merged during interaction
@@ -627,11 +614,6 @@ def run_location_logsums(
         mandatory=False,
     )
 
-    # FIXME - MEMORY HACK - only include columns actually used in spec
-    persons_merged_df = logsum.filter_chooser_columns(
-        persons_merged_df, logsum_settings, model_settings
-    )
-
     logger.info(f"Running {trace_label} with {len(location_sample_df.index)} rows")
 
     choosers = location_sample_df.join(persons_merged_df, how="left")
@@ -691,14 +673,9 @@ def run_location_simulate(
     """
     assert not persons_merged.empty
 
-    # FIXME - MEMORY HACK - only include columns actually used in spec
-    chooser_columns = model_settings.SIMULATE_CHOOSER_COLUMNS
-    # Drop this when PR #1017 is merged
-    if ("household_id" not in chooser_columns) and (
-        "household_id" in persons_merged.columns
-    ):
-        chooser_columns = chooser_columns + ["household_id"]
-    choosers = persons_merged[chooser_columns]
+    # Preprocessors annotate choosers in place.  Use a copy so those temporary
+    # columns do not affect subsequent segments that share persons_merged.
+    choosers = persons_merged.copy()
 
     alt_dest_col_name = model_settings.ALT_DEST_COL_NAME
 

activitysim/abm/models/school_escorting.py

@@ -3,10 +3,12 @@
 from __future__ import annotations
 
 import logging
+import warnings
 from typing import Any, Literal
 
 import numpy as np
 import pandas as pd
+from pydantic import field_validator
 
 from activitysim.abm.models.util import school_escort_tours_trips
 from activitysim.core import (
@@ -391,7 +393,26 @@ class SchoolEscortSettings(BaseLogitComponentSettings, extra="forbid"):
     GENDER_WEIGHT: float = 10.0
     AGE_WEIGHT: float = 1.0
 
-    SIMULATE_CHOOSER_COLUMNS: list[str] | None = None
+    SIMULATE_CHOOSER_COLUMNS: Any | None = None
+    """Was used to help reduce the memory needed for the model.
+
+    This setting is now obsolete and does nothing. Its functionality has been
+    replaced by :func:`activitysim.core.util.drop_unused_columns`.
+
+    .. deprecated:: 1.6
+    """
+
+    @field_validator("SIMULATE_CHOOSER_COLUMNS", mode="before")
+    @classmethod
+    def _deprecate_simulate_chooser_columns(cls, value):
+        if value is not None:
+            warnings.warn(
+                "SIMULATE_CHOOSER_COLUMNS is deprecated and no longer used, "
+                "unused columns are now dropped automatically",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+        return None
 
     SPEC: None = None
     """The school escort model does not use this setting."""
@@ -521,17 +542,6 @@ def school_escorting(
         # else:
         #     locals_dict.pop("_sharrow_skip", None)
 
-        # reduce memory by limiting columns if selected columns are supplied
-        chooser_columns = model_settings.SIMULATE_CHOOSER_COLUMNS
-        if chooser_columns is not None:
-            # Drop this when PR #1017 is merged
-            if ("household_id" not in chooser_columns) and (
-                "household_id" in choosers.columns
-            ):
-                chooser_columns = chooser_columns + ["household_id"]
-            chooser_columns = chooser_columns + participant_columns
-            choosers = choosers[chooser_columns]
-
         # add previous data to stage
         if stage_num >= 1:
             choosers = add_prev_choices_to_choosers(
@@ -622,10 +632,14 @@ def school_escorting(
             )
 
         if stage_num >= 1:
-            choosers["alt"] = choices
-            choosers = choosers.join(alts, how="left", on="alt")
+            # The raw alternative columns are only needed to construct bundle
+            # records.  Do not retain them on the chooser state: the final
+            # stage would otherwise try to join the same columns a second time.
+            bundle_choosers = choosers.assign(alt=choices).join(
+                alts, how="left", on="alt"
+            )
             bundles = create_school_escorting_bundles_table(
-                choosers[choosers["alt"] > 1], tours, stage
+                bundle_choosers[bundle_choosers["alt"] > 1], tours, stage
             )
             escort_bundles.append(bundles)
 

activitysim/abm/models/util/logsums.py

@@ -7,7 +7,6 @@
 import pandas as pd
 
 from activitysim.core import config, expressions, los, simulate, tracing, workflow
-from activitysim.core.configuration import PydanticBase
 from activitysim.core.configuration.logit import (
     TourLocationComponentSettings,
     TourModeComponentSettings,
@@ -16,41 +15,6 @@
 logger = logging.getLogger(__name__)
 
 
-def filter_chooser_columns(
-    choosers, logsum_settings: dict | PydanticBase, model_settings: dict | PydanticBase
-):
-    try:
-        chooser_columns = logsum_settings.LOGSUM_CHOOSER_COLUMNS
-    except AttributeError:
-        chooser_columns = logsum_settings.get("LOGSUM_CHOOSER_COLUMNS", [])
-
-    if (
-        isinstance(model_settings, dict)
-        and "CHOOSER_ORIG_COL_NAME" in model_settings
-        and model_settings["CHOOSER_ORIG_COL_NAME"] not in chooser_columns
-    ):
-        chooser_columns.append(model_settings["CHOOSER_ORIG_COL_NAME"])
-    if (
-        isinstance(model_settings, PydanticBase)
-        and hasattr(model_settings, "CHOOSER_ORIG_COL_NAME")
-        and model_settings.CHOOSER_ORIG_COL_NAME
-        and model_settings.CHOOSER_ORIG_COL_NAME not in chooser_columns
-    ):
-        chooser_columns.append(model_settings.CHOOSER_ORIG_COL_NAME)
-
-    missing_columns = [c for c in chooser_columns if c not in choosers]
-    if missing_columns:
-        logger.debug(
-            "logsum.filter_chooser_columns missing_columns %s" % missing_columns
-        )
-
-    # ignore any columns not appearing in choosers df
-    chooser_columns = [c for c in chooser_columns if c in choosers]
-
-    choosers = choosers[chooser_columns]
-    return choosers
-
-
 def compute_location_choice_logsums(
     state: workflow.State,
     choosers: pd.DataFrame,

activitysim/abm/models/util/tour_destination.py

@@ -637,8 +637,10 @@ def destination_presample(
 
     orig_maz = model_settings.CHOOSER_ORIG_COL_NAME
     assert orig_maz in choosers
-    if ORIG_TAZ not in choosers:
-        choosers[ORIG_TAZ] = network_los.map_maz_to_taz(choosers[orig_maz])
+    # This is the TAZ for the configured tour origin.  A wider chooser table
+    # may already contain a same-named home TAZ, which is incorrect for models
+    # such as at-work subtour destination choice.
+    choosers[ORIG_TAZ] = network_los.map_maz_to_taz(choosers[orig_maz])
 
     # create wrapper with keys for this lookup - in this case there is a HOME_TAZ in the choosers
     # and a DEST_TAZ in the alternatives which get merged during interaction
@@ -691,23 +693,9 @@ def run_destination_sample(
     chunk_size,
     trace_label,
 ):
-    # FIXME - MEMORY HACK - only include columns actually used in spec (omit them pre-merge)
-    chooser_columns = model_settings.SIMULATE_CHOOSER_COLUMNS
-
     # if special person id is passed
     chooser_id_column = model_settings.CHOOSER_ID_COLUMN
 
-    # Drop this when PR #1017 is merged
-    if ("household_id" not in chooser_columns) and (
-        "household_id" in persons_merged.columns
-    ):
-        chooser_columns = chooser_columns + ["household_id"]
-    persons_merged = persons_merged[
-        [c for c in persons_merged.columns if c in chooser_columns]
-    ]
-    tours = tours[
-        [c for c in tours.columns if c in chooser_columns or c == chooser_id_column]
-    ]
     choosers = pd.merge(
         tours, persons_merged, left_on=chooser_id_column, right_index=True, how="left"
     )
@@ -805,11 +793,6 @@ def run_destination_logsums(
 
     chunk_tag = "tour_destination.logsums"
 
-    # FIXME - MEMORY HACK - only include columns actually used in spec
-    persons_merged = logsum.filter_chooser_columns(
-        persons_merged, logsum_settings, model_settings
-    )
-
     # merge persons into tours
     choosers = pd.merge(
         destination_sample,
@@ -872,23 +855,9 @@ def run_destination_simulate(
         coefficients_file_name=model_settings.COEFFICIENTS,
     )
 
-    # FIXME - MEMORY HACK - only include columns actually used in spec (omit them pre-merge)
-    chooser_columns = model_settings.SIMULATE_CHOOSER_COLUMNS
-
     # if special person id is passed
     chooser_id_column = model_settings.CHOOSER_ID_COLUMN
 
-    # Drop this when PR #1017 is merged
-    if ("household_id" not in chooser_columns) and (
-        "household_id" in persons_merged.columns
-    ):
-        chooser_columns = chooser_columns + ["household_id"]
-    persons_merged = persons_merged[
-        [c for c in persons_merged.columns if c in chooser_columns]
-    ]
-    tours = tours[
-        [c for c in tours.columns if c in chooser_columns or c == chooser_id_column]
-    ]
     choosers = pd.merge(
         tours, persons_merged, left_on=chooser_id_column, right_index=True, how="left"
     )

activitysim/abm/models/util/tour_od.py

@@ -736,13 +736,8 @@ def run_od_sample(
         coefficients_file_name=model_settings.COEFFICIENTS,
     )
 
-    choosers = tours
-    # FIXME - MEMORY HACK - only include columns actually used in spec
-    chooser_columns = model_settings.SIMULATE_CHOOSER_COLUMNS
-    # Drop this when PR #1017 is merged
-    if ("household_id" not in chooser_columns) and ("household_id" in choosers.columns):
-        chooser_columns = chooser_columns + ["household_id"]
-    choosers = choosers[chooser_columns]
+    # Preserve the independent-frame behavior of the former column subset.
+    choosers = tours.copy()
 
     # interaction_sample requires that choosers.index.is_monotonic_increasing
     if not choosers.index.is_monotonic_increasing:
@@ -820,11 +815,6 @@ def run_od_logsums(
     dest_id_col = model_settings.DEST_COL_NAME
     tour_od_id_col = get_od_id_col(origin_id_col, dest_id_col)
 
-    # FIXME - MEMORY HACK - only include columns actually used in spec
-    tours_merged_df = logsum.filter_chooser_columns(
-        tours_merged_df, logsum_settings, model_settings
-    )
-
     # merge ods into choosers table
     choosers = od_sample.join(tours_merged_df, how="left")
     choosers[tour_od_id_col] = (
@@ -998,14 +988,9 @@ def run_od_simulate(
     )
 
     # merge persons into tours
-    choosers = tours
-
-    # FIXME - MEMORY HACK - only include columns actually used in spec
-    chooser_columns = model_settings.SIMULATE_CHOOSER_COLUMNS
-    # Drop this when PR #1017 is merged
-    if ("household_id" not in chooser_columns) and ("household_id" in choosers.columns):
-        chooser_columns = chooser_columns + ["household_id"]
-    choosers = choosers[chooser_columns]
+    # Preprocessors may annotate choosers in place; keep those columns local to
+    # this segment instead of mutating the shared tours table.
+    choosers = tours.copy()
 
     # interaction_sample requires that choosers.index.is_monotonic_increasing
     if not choosers.index.is_monotonic_increasing:

activitysim/abm/models/util/tour_scheduling.py

@@ -9,7 +9,7 @@
 from activitysim.abm.models.util import vectorize_tour_scheduling as vts
 from activitysim.core import config, estimation, expressions, simulate, workflow
 
-from .vectorize_tour_scheduling import TourModeComponentSettings, TourSchedulingSettings
+from .vectorize_tour_scheduling import TourSchedulingSettings
 
 logger = logging.getLogger(__name__)
 
@@ -24,29 +24,9 @@ def run_tour_scheduling(
     trace_label: str,
 ):
 
-    if model_settings.LOGSUM_SETTINGS:
-        logsum_settings = TourModeComponentSettings.read_settings_file(
-            state.filesystem,
-            str(model_settings.LOGSUM_SETTINGS),
-            mandatory=False,
-        )
-        logsum_columns = logsum_settings.LOGSUM_CHOOSER_COLUMNS
-    else:
-        logsum_columns = []
-
-    # - filter chooser columns for both logsums and simulate
-    model_columns = model_settings.SIMULATE_CHOOSER_COLUMNS
-    chooser_columns = logsum_columns + [
-        c for c in model_columns if c not in logsum_columns
-    ]
-
-    # Drop this when PR #1017 is merged
-    if ("household_id" not in chooser_columns) and (
-        "household_id" in persons_merged.columns
-    ):
-        chooser_columns = chooser_columns + ["household_id"]
-
-    persons_merged = expressions.filter_chooser_columns(persons_merged, chooser_columns)
+    # The deprecated chooser-column filter returned a new frame.  Retain that
+    # isolation because vectorized scheduling annotates merged chooser data.
+    persons_merged = persons_merged.copy()
 
     timetable = state.get_injectable("timetable")
 

activitysim/abm/models/util/vectorize_tour_scheduling.py

@@ -3,12 +3,14 @@
 from __future__ import annotations
 
 import logging
+import warnings
 from collections import OrderedDict
 from pathlib import Path
 from typing import Any
 
 import numpy as np
 import pandas as pd
+from pydantic import field_validator
 
 from activitysim.abm.models.tour_mode_choice import TourModeComponentSettings
 from activitysim.core import chunk, config, expressions, los, simulate
@@ -43,7 +45,26 @@ class TourSchedulingSettings(LogitComponentSettings, extra="forbid"):
     it is assumed to be an unsegmented preprocessor.  Otherwise, the dict keys
     give the segements.
     """
-    SIMULATE_CHOOSER_COLUMNS: list[str] | None = None
+    SIMULATE_CHOOSER_COLUMNS: Any | None = None
+    """Was used to help reduce the memory needed for the model.
+
+    This setting is now obsolete and does nothing. Its functionality has been
+    replaced by :func:`activitysim.core.util.drop_unused_columns`.
+
+    .. deprecated:: 1.6
+    """
+
+    @field_validator("SIMULATE_CHOOSER_COLUMNS", mode="before")
+    @classmethod
+    def _deprecate_simulate_chooser_columns(cls, value):
+        if value is not None:
+            warnings.warn(
+                "SIMULATE_CHOOSER_COLUMNS is deprecated and no longer used, "
+                "unused columns are now dropped automatically",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+        return None
 
     SPEC_SEGMENTS: dict[str, LogitComponentSettings] = {}