Merge pull request #92 from NatLabRockies/copilot/better-route-filtering-handling

Default to trip-level route filtering; use block-level only when deadhead is requested

Author: dan-mccabe

docs/index.md

@@ -34,6 +34,17 @@ predictor.run(
 )
 ```
 
+By default, route filtering works at the **trip level**, so individual trips on the requested routes are always included even if their block also serves other routes. If you enable deadhead trip estimation, filtering automatically switches to **block level** to ensure complete blocks (see [](prediction) for details):
+
+```python
+predictor.run(
+    date="2023/08/02",
+    routes=["806", "807"],
+    add_mid_block_deadhead=True,
+    add_depot_deadhead=True,
+)
+```
+
 For a full example, see [](examples/Utah_Transit_Agency_example).
 
 ## Available Models

docs/prediction.md

@@ -13,6 +13,25 @@ The full workflow proceeds in four stages:
 
 RouteE-Transit reads a standard GTFS feed directory and loads trips along with their shape traces, stop locations, and stop times. Users can optionally filter to a specific service date and/or a subset of routes. The shape traces and scheduled stop times are used downstream to estimate average speed and distance at the road link level.
 
+### Route filtering
+
+By default, route filtering is **trip-level**: only trips whose `route_short_name` appears in the `routes` list are included, regardless of what other routes share the same GTFS block. This gives the most intuitive results, especially for agencies where interlining is common (e.g., a bus block might serve both route 5 and route 21).
+
+When deadhead trips are requested (`add_mid_block_deadhead=True` or `add_depot_deadhead=True`), filtering automatically switches to **block-level** mode. In block-level mode, entire blocks are excluded if *any* trip in the block belongs to a route not in the requested set. This is necessary because deadhead estimation requires complete blocks. If block-level filtering removes all trips but trip-level filtering would have kept some, the error message will explain this and suggest alternatives (either disabling deadhead or adding the additional interlined routes to the `routes` list).
+
+When using `filter_trips()` directly, you can control this behavior with the `use_block_filter` parameter:
+
+```python
+predictor.load_gtfs_data()
+
+# Trip-level filtering (default) — individual trips on route "5" are
+# always kept, even if their block also serves route "21"
+predictor.filter_trips(routes=["5"])
+
+# Block-level filtering — only blocks that exclusively serve route "5"
+predictor.filter_trips(routes=["5"], use_block_filter=True)
+```
+
 See [](data:gtfs-reqs) for the full list of required GTFS files and fields.
 
 ## 2) Deadhead Trip Inference
@@ -77,6 +96,15 @@ predictor = GTFSEnergyPredictor(
 )
 
 # Option 1: Use the convenience method (recommended)
+# By default, only revenue trips are included (no deadhead).
+trip_results = predictor.run(
+    date="2023/08/02",
+    routes=["205"],
+)
+
+# Option 2: Include deadhead trips and HVAC impacts
+# When deadhead is enabled with route filtering, block-level filtering
+# is used automatically to ensure complete blocks.
 trip_results = predictor.run(
     date="2023/08/02",
     routes=["205"],
@@ -85,7 +113,7 @@ trip_results = predictor.run(
     add_hvac=True,
 )
 
-# Option 2: Step-by-step processing for more control
+# Option 3: Step-by-step processing for more control
 predictor.load_gtfs_data()
 predictor.filter_trips(date="2023/08/02", routes=["205"])
 predictor.add_mid_block_deadhead()  # Between-trip deadhead

routee/transit/predictor.py

@@ -228,8 +228,8 @@ def run(
         date: str | None = None,
         routes: list[str] | None = None,
         # Processing options
-        add_mid_block_deadhead: bool = True,
-        add_depot_deadhead: bool = True,
+        add_mid_block_deadhead: bool = False,
+        add_depot_deadhead: bool = False,
         # Energy prediction options
         add_hvac: bool = True,
         save_results: bool = True,
@@ -254,11 +254,16 @@ def run(
             If None, all trips across all service dates are included.
         routes : list[str], optional
             Filter trips to specific route IDs. If None, all routes are included.
-        add_mid_block_deadhead : bool, default=True
+        add_mid_block_deadhead : bool, default=False
             Whether to add deadhead trips between consecutive revenue trips.
-        add_depot_deadhead : bool, default=True
+            When True and ``routes`` is specified, block-level filtering is used
+            to ensure only blocks that exclusively serve the selected routes are
+            included (required for correct deadhead estimation).
+        add_depot_deadhead : bool, default=False
             Whether to add deadhead trips from/to depots at start/end of blocks.
             Requires depot_path to be set during initialization.
+            When True and ``routes`` is specified, block-level filtering is used
+            (see ``add_mid_block_deadhead``).
         add_hvac : bool, default=True
             Whether to add HVAC energy consumption based on ambient temperature.
         save_results : bool, default=True
@@ -306,8 +311,16 @@ def run(
         self.load_gtfs_data()
 
         # Step 2: Filter trips if requested
+        # Use block-level filtering when deadhead trips are requested, because
+        # deadhead estimation requires complete blocks.  Otherwise, use the
+        # more intuitive trip-level filtering.
+        needs_deadhead = add_mid_block_deadhead or add_depot_deadhead
         if date is not None or routes is not None:
-            self.filter_trips(date=date, routes=routes)
+            self.filter_trips(
+                date=date,
+                routes=routes,
+                use_block_filter=needs_deadhead and routes is not None,
+            )
 
         # Add start time, end time, and duration of each trip
         self.add_trip_times()
@@ -553,25 +566,44 @@ def filter_trips(
         self,
         date: str | None = None,
         routes: list[str] | None = None,
+        use_block_filter: bool = False,
     ) -> "GTFSEnergyPredictor":
         """
         Filter trips by date and/or routes.
 
         This method can be called after load_gtfs_data() to restrict the analysis
         to specific dates or routes. Can be called multiple times to refine filters.
 
-        Args:
-            date: Date to filter trips (format: "YYYY-MM-DD" or datetime object).
-                If None, keeps all currently loaded trips.
-            routes: List of route_short_name values to filter by.
-                If None, keeps all currently loaded routes.
-
-        Returns:
-            Self for method chaining
+        Parameters
+        ----------
+        date : str, optional
+            Date to filter trips (format: "YYYY-MM-DD" or datetime object).
+            If None, keeps all currently loaded trips.
+        routes : list[str], optional
+            List of route_short_name values to filter by.
+            If None, keeps all currently loaded routes.
+        use_block_filter : bool, default=False
+            When True, uses block-level filtering via
+            ``filter_blocks_by_route`` with ``route_method="exclusive"``.
+            This means entire blocks are excluded if any trip in the block
+            belongs to a route not in ``routes``. This is appropriate when
+            deadhead trips are being estimated, because we need complete
+            blocks.  When False (the default), trips are filtered purely
+            at the trip level so that individual trips on the requested
+            routes are always included regardless of what other routes
+            share the same block.
 
-        Raises:
-            RuntimeError: If GTFS data hasn't been loaded yet
-            ValueError: If no trips match the specified filters
+        Returns
+        -------
+        GTFSEnergyPredictor
+            Self for method chaining.
+
+        Raises
+        ------
+        RuntimeError
+            If GTFS data hasn't been loaded yet.
+        ValueError
+            If no trips match the specified filters.
         """
         if self.feed is None or self.trips.empty:
             raise RuntimeError("Must call load_gtfs_data() before filtering trips")
@@ -588,14 +620,40 @@ def filter_trips(
 
         # Filter by routes
         if routes is not None:
-            self.trips = filter_blocks_by_route(
-                trips=self.trips,
-                routes=routes,
-                route_column="route_short_name",
-                route_method="exclusive",
-            )
-            if len(self.trips) == 0:
-                raise ValueError("No trips found for the selected routes and date.")
+            if use_block_filter:
+                pre_filter_trips = self.trips
+                self.trips = filter_blocks_by_route(
+                    trips=self.trips,
+                    routes=routes,
+                    route_column="route_short_name",
+                    route_method="exclusive",
+                )
+                if len(self.trips) == 0:
+                    # Check whether trip-level filtering would have kept any
+                    # trips.  This tells the user whether the issue is that
+                    # no trips match the routes at all, or that block-level
+                    # filtering is too restrictive.
+                    trip_level_count = int(
+                        pre_filter_trips["route_short_name"].isin(routes).sum()
+                    )
+                    if trip_level_count > 0:
+                        raise ValueError(
+                            f"No trips remain after block-level route filtering, "
+                            f"but {trip_level_count} trip(s) match at the trip "
+                            f"level. This can happen when blocks contain trips "
+                            f"from routes not in the requested set (e.g. "
+                            f"interlined routes). Consider running without "
+                            f"deadhead trips to use trip-level filtering, or "
+                            f"add the additional routes to the 'routes' "
+                            f"parameter."
+                        )
+                    raise ValueError("No trips found for the selected routes and date.")
+            else:
+                self.trips = self.trips[
+                    self.trips["route_short_name"].isin(routes)
+                ].copy()
+                if len(self.trips) == 0:
+                    raise ValueError("No trips found for the selected routes and date.")
 
         # Update shapes to match filtered trips
         shape_ids = self.trips.shape_id.unique()

tests/test_predictor.py

@@ -46,8 +46,7 @@ def test_load_gtfs_data(self, mock_from_dir: MagicMock) -> None:
         self.assertEqual(self.predictor.trips.iloc[0]["trip_id"], "T1")
         self.assertEqual(len(self.predictor.shapes), 1)
 
-    @patch("routee.transit.predictor.filter_blocks_by_route")
-    def test_filter_trips(self, mock_filter: MagicMock) -> None:
+    def test_filter_trips_by_date(self) -> None:
         # Setup pre-loaded state
         self.predictor.feed = MagicMock()
         self.predictor.feed.get_service_ids_from_date.return_value = ["S1"]
@@ -65,6 +64,90 @@ def test_filter_trips(self, mock_filter: MagicMock) -> None:
         self.assertEqual(len(self.predictor.trips), 1)
         self.assertEqual(self.predictor.trips.iloc[0]["service_id"], "S1")
 
+    def test_filter_trips_trip_level_default(self) -> None:
+        """Trip-level filtering keeps individual trips even if the block has other routes."""
+        self.predictor.feed = MagicMock()
+        self.predictor.trips = pd.DataFrame(
+            {
+                "trip_id": ["T1", "T2", "T3"],
+                "block_id": ["B1", "B1", "B2"],
+                "route_short_name": ["R1", "R2", "R1"],
+                "service_id": ["S1", "S1", "S1"],
+                "shape_id": ["SH1", "SH2", "SH3"],
+            }
+        )
+        self.predictor.feed.shapes = pd.DataFrame({"shape_id": ["SH1", "SH2", "SH3"]})
+
+        self.predictor.filter_trips(routes=["R1"])
+
+        # Should keep T1 and T3 (both on R1) even though T1's block also has R2
+        self.assertEqual(len(self.predictor.trips), 2)
+        self.assertListEqual(
+            sorted(self.predictor.trips["trip_id"].tolist()), ["T1", "T3"]
+        )
+
+    def test_filter_trips_block_level(self) -> None:
+        """Block-level filtering excludes blocks that contain trips from other routes."""
+        self.predictor.feed = MagicMock()
+        self.predictor.trips = pd.DataFrame(
+            {
+                "trip_id": ["T1", "T2", "T3"],
+                "block_id": ["B1", "B1", "B2"],
+                "route_short_name": ["R1", "R2", "R1"],
+                "service_id": ["S1", "S1", "S1"],
+                "shape_id": ["SH1", "SH2", "SH3"],
+            }
+        )
+        self.predictor.feed.shapes = pd.DataFrame({"shape_id": ["SH1", "SH2", "SH3"]})
+
+        self.predictor.filter_trips(routes=["R1"], use_block_filter=True)
+
+        # Block B1 has R2 so it is excluded entirely; only B2/T3 remains
+        self.assertEqual(len(self.predictor.trips), 1)
+        self.assertEqual(self.predictor.trips.iloc[0]["trip_id"], "T3")
+
+    def test_filter_trips_block_level_error_message(self) -> None:
+        """Block-level filtering gives a helpful error when all trips are excluded."""
+        self.predictor.feed = MagicMock()
+        self.predictor.trips = pd.DataFrame(
+            {
+                "trip_id": ["T1", "T2"],
+                "block_id": ["B1", "B1"],
+                "route_short_name": ["R1", "R2"],
+                "service_id": ["S1", "S1"],
+                "shape_id": ["SH1", "SH2"],
+            }
+        )
+        self.predictor.feed.shapes = pd.DataFrame({"shape_id": ["SH1", "SH2"]})
+
+        with self.assertRaises(ValueError) as ctx:
+            self.predictor.filter_trips(routes=["R1"], use_block_filter=True)
+
+        self.assertIn("block-level", str(ctx.exception))
+        self.assertIn("interlined", str(ctx.exception))
+        # Should mention the count of trip-level matches
+        self.assertIn("1 trip(s) match at the trip level", str(ctx.exception))
+
+    def test_filter_trips_block_level_no_trips_at_all(self) -> None:
+        """Block-level filtering with no matching trips gives a generic error."""
+        self.predictor.feed = MagicMock()
+        self.predictor.trips = pd.DataFrame(
+            {
+                "trip_id": ["T1", "T2"],
+                "block_id": ["B1", "B1"],
+                "route_short_name": ["R1", "R2"],
+                "service_id": ["S1", "S1"],
+                "shape_id": ["SH1", "SH2"],
+            }
+        )
+        self.predictor.feed.shapes = pd.DataFrame({"shape_id": ["SH1", "SH2"]})
+
+        with self.assertRaises(ValueError) as ctx:
+            self.predictor.filter_trips(routes=["R99"], use_block_filter=True)
+
+        # No trip-level matches either, so the generic error should be used
+        self.assertIn("No trips found", str(ctx.exception))
+
     def test_add_trip_times(self) -> None:
         # Setup pre-loaded state
         self.predictor.feed = MagicMock()