From 609417fe018766194b10a877a9b2aa429cadd535 Mon Sep 17 00:00:00 2001 From: Will Alexander Date: Wed, 24 Jun 2026 15:46:30 -0500 Subject: [PATCH 1/4] Stable sort for school escorting --- activitysim/abm/models/school_escorting.py | 11 +++++++---- .../abm/models/util/school_escort_tours_trips.py | 12 +++++++----- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/activitysim/abm/models/school_escorting.py b/activitysim/abm/models/school_escorting.py index d24d52a41f..421097e240 100644 --- a/activitysim/abm/models/school_escorting.py +++ b/activitysim/abm/models/school_escorting.py @@ -85,13 +85,15 @@ def determine_escorting_participants( ) chaperones["chaperone_num"] = ( - chaperones.sort_values("chaperone_weight", ascending=False) + chaperones.sort_values("chaperone_weight", ascending=False, kind="stable") .groupby("household_id") .cumcount() + 1 ) escortees["escortee_num"] = ( - escortees.sort_values("age", ascending=True).groupby("household_id").cumcount() + escortees.sort_values("age", ascending=True, kind="stable") + .groupby("household_id") + .cumcount() + 1 ) @@ -247,7 +249,7 @@ def create_school_escorting_bundles_table(choosers, tours, stage): ) # each chauffeur option has ride share or pure escort - bundles["chauf_num"] = np.ceil(bundles["chauf_type_num"].div(2)).astype(int) + bundles["chauf_num"] = np.ceil(bundles["chauf_type_num"].div(2)).astype("int64") # getting bundle chauffeur id based on the chauffeur num bundles["chauf_id"] = -1 @@ -257,7 +259,7 @@ def create_school_escorting_bundles_table(choosers, tours, stage): choosers["chauf_id" + str(i)], bundles["chauf_id"], ) - bundles["chauf_id"] = bundles["chauf_id"].astype(int) + bundles["chauf_id"] = bundles["chauf_id"].astype("int64") assert ( bundles["chauf_id"] > 0 ).all(), "Invalid chauf_id's for school escort bundles!" @@ -586,6 +588,7 @@ def school_escorting( by=["household_id", "school_escort_direction"], ascending=[True, False], inplace=True, + kind="stable", ) school_escort_tours = school_escort_tours_trips.create_pure_school_escort_tours( diff --git a/activitysim/abm/models/util/school_escort_tours_trips.py b/activitysim/abm/models/util/school_escort_tours_trips.py index 43eed64bb1..5e8e984cd5 100644 --- a/activitysim/abm/models/util/school_escort_tours_trips.py +++ b/activitysim/abm/models/util/school_escort_tours_trips.py @@ -7,7 +7,7 @@ from activitysim.abm.models.school_escorting import NUM_ESCORTEES from activitysim.abm.models.util import canonical_ids -from activitysim.core import workflow +from activitysim.core import estimation, workflow from activitysim.core.util import reindex logger = logging.getLogger(__name__) @@ -78,7 +78,7 @@ def join_attributes(df, column_names): series = ( df[col] .fillna(-1) - .astype(int) + .astype("int64") .astype(str) .replace("-1", "", regex=False) ) @@ -331,7 +331,7 @@ def create_chauf_trip_table(bundles): def create_chauf_escort_trips(bundles): chauf_trip_bundles = create_chauf_trip_table(bundles.copy()) - chauf_trip_bundles["tour_id"] = bundles["chauf_tour_id"].astype(int) + chauf_trip_bundles["tour_id"] = bundles["chauf_tour_id"].astype("int64") # departure time is the first school start in the outbound school_escort_direction and the last school end in the inbound school_escort_direction starts = ( @@ -651,7 +651,7 @@ def process_tours_after_escorting_model(state: workflow.State, escort_bundles, t num_escortees = ( escort_bundles.drop_duplicates("chauf_tour_id") .set_index("chauf_tour_id")["num_escortees"] - .astype(int) + .astype("int64") ) tours.loc[num_escortees.index, "num_escortees"] = num_escortees @@ -921,7 +921,9 @@ def create_pure_school_escort_tours(state: workflow.State, bundles): pe_tours["school_escort_direction"] == "inbound", "pure_escort", pd.NA ) - pe_tours = pe_tours.sort_values(by=["household_id", "person_id", "start"]) + pe_tours = pe_tours.sort_values( + by=["household_id", "person_id", "start"], kind="stable" + ) # finding what the next start time for that person for scheduling pe_tours["next_pure_escort_start"] = ( From 91665449355eef2099e2492ddf19036dabb51acf Mon Sep 17 00:00:00 2001 From: Will Alexander Date: Wed, 24 Jun 2026 15:51:41 -0500 Subject: [PATCH 2/4] Remove unneeded import --- activitysim/abm/models/util/school_escort_tours_trips.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activitysim/abm/models/util/school_escort_tours_trips.py b/activitysim/abm/models/util/school_escort_tours_trips.py index 5e8e984cd5..7c9e997fc0 100644 --- a/activitysim/abm/models/util/school_escort_tours_trips.py +++ b/activitysim/abm/models/util/school_escort_tours_trips.py @@ -7,7 +7,7 @@ from activitysim.abm.models.school_escorting import NUM_ESCORTEES from activitysim.abm.models.util import canonical_ids -from activitysim.core import estimation, workflow +from activitysim.core import workflow from activitysim.core.util import reindex logger = logging.getLogger(__name__) From 96b269b5dd5543daf574354173f9f984375ddd4c Mon Sep 17 00:00:00 2001 From: Jeffrey Newman Date: Thu, 25 Jun 2026 14:47:15 -0500 Subject: [PATCH 3/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- activitysim/abm/models/school_escorting.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activitysim/abm/models/school_escorting.py b/activitysim/abm/models/school_escorting.py index 421097e240..4c5c38f566 100644 --- a/activitysim/abm/models/school_escorting.py +++ b/activitysim/abm/models/school_escorting.py @@ -249,7 +249,7 @@ def create_school_escorting_bundles_table(choosers, tours, stage): ) # each chauffeur option has ride share or pure escort - bundles["chauf_num"] = np.ceil(bundles["chauf_type_num"].div(2)).astype("int64") + bundles["chauf_num"] = ((bundles["chauf_type_num"] + 1) // 2).astype("int64") # getting bundle chauffeur id based on the chauffeur num bundles["chauf_id"] = -1 From 0082f014f4c29453561f74931155cd4394ce346a Mon Sep 17 00:00:00 2001 From: Jeff Newman Date: Tue, 4 Aug 2026 21:46:58 -0500 Subject: [PATCH 4/4] Make school escort assignments deterministic Use stable semantic tie-breakers for participants, bundles, and tours while preserving 64-bit IDs. Add validation and regression coverage for input-order independence, duplicate bundle keys, tied travel times, and large identifiers. --- activitysim/abm/models/school_escorting.py | 74 ++++- .../models/util/school_escort_tours_trips.py | 5 +- .../util/test/test_school_escorting_utils.py | 258 +++++++++++++++++- 3 files changed, 319 insertions(+), 18 deletions(-) diff --git a/activitysim/abm/models/school_escorting.py b/activitysim/abm/models/school_escorting.py index 4c5c38f566..e5378a263a 100644 --- a/activitysim/abm/models/school_escorting.py +++ b/activitysim/abm/models/school_escorting.py @@ -85,13 +85,15 @@ def determine_escorting_participants( ) chaperones["chaperone_num"] = ( - chaperones.sort_values("chaperone_weight", ascending=False, kind="stable") + chaperones.sort_values( + ["chaperone_weight", "person_id"], ascending=[False, True] + ) .groupby("household_id") .cumcount() + 1 ) escortees["escortee_num"] = ( - escortees.sort_values("age", ascending=True, kind="stable") + escortees.sort_values([age_col, "person_id"], ascending=[True, True]) .groupby("household_id") .cumcount() + 1 @@ -280,9 +282,13 @@ def create_school_escorting_bundles_table(choosers, tours, stage): school_time_cols = [ "time_home_to_school" + str(i) for i in range(1, NUM_ESCORTEES + 1) ] - bundles["outbound_order"] = list(bundles[school_time_cols].values.argsort() + 1) + # Child number is the deterministic tie-breaker when siblings have the same + # home-to-school time, so preserve the order of the child-number columns. + bundles["outbound_order"] = list( + bundles[school_time_cols].values.argsort(kind="stable") + 1 + ) bundles["inbound_order"] = list( - (-1 * bundles[school_time_cols]).values.argsort() + 1 + (-1 * bundles[school_time_cols]).values.argsort(kind="stable") + 1 ) # inbound gets reverse order bundles["child_order"] = np.where( bundles["school_escort_direction"] == "outbound", @@ -316,6 +322,54 @@ def create_school_escorting_bundles_table(choosers, tours, stage): return bundles +def assign_school_escort_bundle_ids(escort_bundles: pd.DataFrame) -> pd.DataFrame: + """Sort bundles by semantic keys and assign deterministic, unique IDs.""" + bundle_key_columns = [ + "household_id", + "school_escort_direction", + "bundle_num", + ] + duplicate_keys = escort_bundles.duplicated(bundle_key_columns, keep=False) + if duplicate_keys.any(): + duplicates = escort_bundles.loc[duplicate_keys, bundle_key_columns] + raise ValueError(f"Duplicate school escort bundle keys:\n{duplicates}") + + # Inbound bundles were historically appended first and therefore received + # the lower IDs. Use an explicit direction rank to preserve that behavior + # without depending on categorical or input row ordering. + direction = escort_bundles["school_escort_direction"] + direction_order = np.select( + [direction == "inbound", direction == "outbound"], [0, 1], default=-1 + ) + if (direction_order < 0).any(): + invalid_directions = direction[direction_order < 0].unique().tolist() + raise ValueError( + f"Invalid school escort bundle directions: {invalid_directions}" + ) + + escort_bundles = ( + escort_bundles.assign(_school_escort_direction_order=direction_order) + .sort_values( + by=[ + "household_id", + "_school_escort_direction_order", + "bundle_num", + ] + ) + .drop(columns="_school_escort_direction_order") + ) + escort_bundles["bundle_id"] = ( + escort_bundles["household_id"].astype("int64") * 10 + + escort_bundles.groupby("household_id").cumcount() + + 1 + ).astype("int64") + + if not escort_bundles["bundle_id"].is_unique: + raise ValueError("Generated school escort bundle IDs are not unique") + + return escort_bundles + + class SchoolEscortSettings(BaseLogitComponentSettings, extra="forbid"): """ Settings for the `telecommute_frequency` component. @@ -579,17 +633,7 @@ def school_escorting( # Only want to create bundles and tours and trips if at least one household has school escorting if len(escort_bundles) > 0: - escort_bundles["bundle_id"] = ( - escort_bundles["household_id"] * 10 - + escort_bundles.groupby("household_id").cumcount() - + 1 - ) - escort_bundles.sort_values( - by=["household_id", "school_escort_direction"], - ascending=[True, False], - inplace=True, - kind="stable", - ) + escort_bundles = assign_school_escort_bundle_ids(escort_bundles) school_escort_tours = school_escort_tours_trips.create_pure_school_escort_tours( state, escort_bundles diff --git a/activitysim/abm/models/util/school_escort_tours_trips.py b/activitysim/abm/models/util/school_escort_tours_trips.py index 7c9e997fc0..c95436dbf0 100644 --- a/activitysim/abm/models/util/school_escort_tours_trips.py +++ b/activitysim/abm/models/util/school_escort_tours_trips.py @@ -921,8 +921,11 @@ def create_pure_school_escort_tours(state: workflow.State, bundles): pe_tours["school_escort_direction"] == "inbound", "pure_escort", pd.NA ) + if not pe_tours["bundle_id"].is_unique: + raise ValueError("Pure school escort bundle IDs are not unique") + pe_tours = pe_tours.sort_values( - by=["household_id", "person_id", "start"], kind="stable" + by=["household_id", "person_id", "start", "bundle_id"] ) # finding what the next start time for that person for scheduling diff --git a/activitysim/abm/models/util/test/test_school_escorting_utils.py b/activitysim/abm/models/util/test/test_school_escorting_utils.py index 5a007952a5..508d0e5bf7 100644 --- a/activitysim/abm/models/util/test/test_school_escorting_utils.py +++ b/activitysim/abm/models/util/test/test_school_escorting_utils.py @@ -1,15 +1,29 @@ +from __future__ import annotations + # ActivitySim # See full license in LICENSE.txt. import os from ast import literal_eval -import pandas as pd + import numpy as np +import pandas as pd import pandas.testing as pdt +import pytest +import activitysim.abm.models.school_escorting as school_escorting +from activitysim.abm.models.school_escorting import ( + SchoolEscortSettings, + assign_school_escort_bundle_ids, + create_school_escorting_bundles_table, + determine_escorting_participants, +) +from activitysim.abm.models.util import canonical_ids from activitysim.abm.models.util.school_escort_tours_trips import ( create_bundle_attributes, - create_child_escorting_stops, + create_chauf_escort_trips, create_chauf_trip_table, + create_child_escorting_stops, + create_pure_school_escort_tours, ) @@ -70,6 +84,246 @@ def test_create_child_escorting_stops(): pdt.assert_frame_equal(escortee_trips, escortee_trips_expected) +def _make_escorting_persons(): + """Create households with tied and untied participant rankings.""" + return pd.DataFrame( + { + "person_id": [101, 102, 103, 104, 201, 202, 203, 204], + "household_id": [1, 1, 1, 1, 2, 2, 2, 2], + "ptype": [1, 1, 8, 8, 1, 4, 8, 8], + "sex": [1, 1, 2, 2, 1, 2, 1, 1], + "age": [40, 40, 9, 9, 45, 42, 7, 12], + "is_student": [False, False, True, True, False, False, True, True], + "cdap_activity": ["M"] * 8, + } + ).set_index("person_id") + + +def _participant_assignments(persons): + """Return only the participant ID columns produced for test households.""" + choosers = pd.DataFrame({"household_id": [1, 2], "home_zone_id": [5, 6]}).set_index( + "household_id" + ) + model_settings = SchoolEscortSettings(ALTS="dummy") + choosers, participant_columns = determine_escorting_participants( + choosers, persons, model_settings + ) + return choosers[participant_columns] + + +def test_determine_escorting_participants_order_independent(): + """Participant assignment is independent of input order and MP slicing.""" + persons = _make_escorting_persons() + baseline = _participant_assignments(persons) + + candidates = [ + persons.iloc[::-1], + persons.iloc[[3, 0, 2, 1, 7, 4, 6, 5]], + persons[persons["household_id"] == 1], + persons[persons["household_id"] == 2], + ] + for reordered in candidates: + result = _participant_assignments(reordered) + common = result.index.intersection(baseline.index) + pdt.assert_frame_equal( + result.loc[common].sort_index(), baseline.loc[common].sort_index() + ) + + +def test_determine_escorting_participants_ranking_and_tie_breaks(): + """Weights and ages rank first, with person ID resolving ties.""" + assignments = _participant_assignments(_make_escorting_persons()) + + assert assignments.loc[1, "chauf_id1"] == 101 + assert assignments.loc[1, "chauf_id2"] == 102 + assert assignments.loc[1, "child_id1"] == 103 + assert assignments.loc[1, "child_id2"] == 104 + + assert assignments.loc[2, "chauf_id1"] == 202 + assert assignments.loc[2, "chauf_id2"] == 201 + assert assignments.loc[2, "child_id1"] == 203 + assert assignments.loc[2, "child_id2"] == 204 + + +def _make_escort_bundles_for_ids(): + """Create bundle rows whose input order should not affect their IDs.""" + direction_dtype = pd.CategoricalDtype(["outbound", "inbound"]) + bundles = pd.DataFrame( + { + "household_id": [100, 100, 100, 200, 200], + "school_escort_direction": [ + "inbound", + "inbound", + "outbound", + "inbound", + "outbound", + ], + "bundle_num": [1, 2, 1, 1, 1], + } + ) + bundles["school_escort_direction"] = bundles["school_escort_direction"].astype( + direction_dtype + ) + return bundles + + +def test_assign_school_escort_bundle_ids_order_independent(): + """Semantic bundle keys produce stable IDs for arbitrarily ordered rows.""" + bundles = _make_escort_bundles_for_ids() + baseline = assign_school_escort_bundle_ids(bundles) + shuffled = assign_school_escort_bundle_ids(bundles.sample(frac=1, random_state=19)) + + keys = ["household_id", "school_escort_direction", "bundle_num"] + pdt.assert_series_equal( + baseline.set_index(keys)["bundle_id"].sort_index(), + shuffled.set_index(keys)["bundle_id"].sort_index(), + ) + assert baseline["bundle_id"].is_unique + + +def test_assign_school_escort_bundle_ids_rejects_duplicate_keys(): + """Duplicate semantic bundle keys fail instead of using input order.""" + bundles = _make_escort_bundles_for_ids() + bundles = pd.concat([bundles, bundles.iloc[[0]]], ignore_index=True) + + with pytest.raises(ValueError, match="Duplicate school escort bundle keys"): + assign_school_escort_bundle_ids(bundles) + + +@pytest.mark.parametrize("stage", ["outbound_cond", "inbound"]) +def test_create_bundles_orders_tied_escortees_and_uses_int64(monkeypatch, stage): + """Equal travel times use child number while large IDs remain int64.""" + monkeypatch.setattr(school_escorting, "NUM_ESCORTEES", 3) + monkeypatch.setattr(school_escorting, "NUM_CHAPERONES", 2) + + choosers = pd.DataFrame( + { + "household_id": [100], + "home_zone_id": [5], + "nbundles": [1], + "bundle1": [1], + "bundle2": [1], + "bundle3": [0], + "child_id1": [3_000_000_003], + "child_id2": [3_000_000_004], + "child_id3": [0], + "chauf1": [2], + "chauf2": [2], + "chauf3": [0], + "chauf_id1": [3_000_000_001], + "chauf_id2": [3_000_000_002], + "time_home_to_school1": [10.0], + "time_home_to_school2": [10.0], + "time_home_to_school3": [99.0], + "alt": [2], + "Description": ["test"], + } + ).set_index("household_id") + tours = pd.DataFrame( + { + "tour_id": [5_000_000_011, 5_000_000_031, 5_000_000_041], + "person_id": [3_000_000_001, 3_000_000_003, 3_000_000_004], + "tour_type": ["work", "school", "school"], + "tour_num": [1, 1, 1], + "tour_category": ["mandatory"] * 3, + "start": [9, 8, 8], + "end": [17, 15, 15], + "destination": [30, 20, 21], + "origin": [5, 5, 5], + } + ).set_index("tour_id") + + bundles = create_school_escorting_bundles_table(choosers, tours, stage) + + assert list(bundles["child_order"].iloc[0]) == [1, 2, 3] + assert bundles["escortees"].iloc[0] == "3000000003_3000000004" + assert bundles["chauf_num"].dtype == np.dtype("int64") + assert bundles["chauf_id"].dtype == np.dtype("int64") + + +class _FakeState: + """Provide the tours table used only for categorical dtypes.""" + + def __init__(self, tours): + self._tours = tours + + def get_dataframe(self, name): + assert name == "tours" + return self._tours + + +def _fake_set_tour_index(state, tours, is_school_escorting=False, **kwargs): + """Assign order-sensitive IDs so a missing tie-break is observable.""" + tours["tour_id"] = tours["person_id"] * 100 + tours["tour_type_num"] + tours.set_index("tour_id", inplace=True) + return tours + + +def _make_pure_escort_bundles(): + """Create two tied pure-escort tours for one chauffeur.""" + return pd.DataFrame( + { + "bundle_id": [1001, 1002], + "household_id": [100, 100], + "chauf_id": [1001, 1001], + "escort_type": ["pure_escort", "pure_escort"], + "school_escort_direction": ["outbound", "outbound"], + "home_zone_id": [5, 5], + "school_destinations": ["20", "21"], + "school_starts": ["8", "8"], + "school_ends": ["15", "15"], + } + ) + + +def _run_pure_escort(bundles, monkeypatch): + """Create pure-escort tours with a minimal workflow state.""" + monkeypatch.setattr(canonical_ids, "set_tour_index", _fake_set_tour_index) + tours_for_dtype = pd.DataFrame( + { + "tour_category": pd.Categorical(["non_mandatory"]), + "tour_type": pd.Categorical(["escort"]), + } + ) + result = create_pure_school_escort_tours( + _FakeState(tours_for_dtype), bundles.copy() + ) + return result.reset_index().set_index("bundle_id") + + +def test_create_pure_school_escort_tours_order_independent(monkeypatch): + """Bundle ID resolves tied start times independently of input order.""" + bundles = _make_pure_escort_bundles() + baseline = _run_pure_escort(bundles, monkeypatch) + reversed_input = _run_pure_escort(bundles.iloc[::-1], monkeypatch) + + columns = [ + "tour_id", + "tour_num", + "tour_type_num", + "next_pure_escort_start", + ] + pdt.assert_frame_equal( + reversed_input[columns].sort_index(), baseline[columns].sort_index() + ) + assert baseline.loc[1001, "tour_num"] == 1 + assert baseline.loc[1002, "tour_num"] == 2 + + +def test_create_chauf_escort_trips_uses_int64_tour_ids(): + """Chauffeur trips retain tour IDs larger than a signed 32-bit integer.""" + data_dir = os.path.join(os.path.dirname(__file__), "data") + bundles = pd.read_pickle( + os.path.join(data_dir, "create_chauf_trip_table__input.pkl") + ) + bundles["chauf_tour_id"] += 3_000_000_000 + + trips = create_chauf_escort_trips(bundles) + + assert trips["tour_id"].dtype == np.dtype("int64") + assert trips["tour_id"].min() > np.iinfo(np.int32).max + + if __name__ == "__main__": test_create_bundle_attributes() test_create_chauf_trip_table()