Add MPGe column to results#78
Conversation
…powertrain to compass)
There was a problem hiding this comment.
Pull request overview
This PR adds an MPGe metric to trip-level energy prediction outputs to enable powertrain-agnostic efficiency comparisons, and also trims some trip-level columns. It additionally refactors the GTFS feed gathering script and adds a --feed_ids option for targeted Mobility Database processing.
Changes:
- Add per-trip
mpgecalculation (via GGE conversion factors) and drop some trip-level GTFS metadata columns from prediction outputs. - Refactor
scripts/feeds/gather_feeds.pyinto smaller functions and add--feed_idsfor selecting specific Mobility Database feeds. - Add
GtfsExtractor.query_mdb_feed()to support querying a single feed directly by ID.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| scripts/feeds/gather_feeds.py | Refactors feed/dataset collection flow; adds --feed_ids filtering and writes feed/dataset summary CSVs. |
| scripts/feeds/extract_static_gtfs.py | Adds query_mdb_feed() helper to fetch one feed record by ID. |
| routee/transit/predictor.py | Adds mpge to trip-level results and drops selected trip metadata columns; updates energy model config with GGE conversion factors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Compute MPGe (miles per gallon equivalent) — a common efficiency | ||
| # metric across all fuel types using EPA GGE conversion factors | ||
| gge_per_unit = float(model_config["gge_per_unit"]) | ||
| gge_consumed = trip_results["energy_used"] * gge_per_unit | ||
| trip_results["mpge"] = trip_results["miles"] / gge_consumed | ||
| # Replace inf/negative with NaN for trips with zero or invalid energy | ||
| trip_results.loc[gge_consumed <= 0, "mpge"] = float("nan") | ||
|
|
||
| # Drop columns that are not useful in trip-level output | ||
| trip_results = trip_results.drop( | ||
| columns=["service_id", "route_short_name", "route_desc", "route_type"], | ||
| errors="ignore", | ||
| ) |
There was a problem hiding this comment.
mpge is newly computed here but there are no unit tests exercising predict_energy() output (including the zero/negative-energy handling that should yield NaN). Since routee/transit/predictor.py already has a test module (tests/test_predictor.py), add coverage for MPGe calculation (at least one BEB and one diesel case) and confirm the dropped columns are absent/preserved as expected.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
|
@dan-mccabe I've opened a new pull request, #79, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: dan-mccabe <25916826+dan-mccabe@users.noreply.github.com>
Add unit tests for MPGe calculation in `predict_energy()`
Uh oh!
There was an error while loading. Please reload this page.