-
Notifications
You must be signed in to change notification settings - Fork 118
Trip Scheduling Choice -- Same Results Single Process and Multi-Process #1005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
single process run with
…ctivitysim into trip_scheduling_rng_fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR makes several updates to the trip scheduling and departure choice models and their test configurations:
- Migrates
TripSchedulingChoiceSettingsto inherit fromLogitComponentSettingsinstead of defining fields manually - Updates configuration files to use standardized field names (
SPECinstead ofSPECIFICATION,preprocessorinstead ofPREPROCESSOR) - Un-comments and updates trip scheduling and departure choice tests to be functional again
- Adds new test configuration files and fixtures
- Removes placeholder
.gitignorefiles from output directories - Adds a new simulation script and test configuration for the prototype_arc example
Reviewed Changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| activitysim/examples/prototype_arc/test/simulation.py | Adds a command-line script to run simulations via ActivitySim's CLI |
| activitysim/examples/prototype_arc/test/output/trace/.gitignore | Removes .gitignore for trace output directory |
| activitysim/examples/prototype_arc/test/output/cache/.gitignore | Removes .gitignore for cache output directory |
| activitysim/examples/prototype_arc/test/output/.gitignore | Removes .gitignore for main output directory |
| activitysim/examples/prototype_arc/configs/trip_scheduling_choice.yaml | Updates to use standard field names and adds compute_settings with protect_columns |
| activitysim/examples/prototype_arc/configs/settings.yaml | Adds test configuration settings including sample size, model settings check flag, and multiprocessing settings |
| activitysim/abm/test/test_misc/test_trip_scheduling_choice.py | Un-comments and updates tests with proper imports and fixtures, including state initialization |
| activitysim/abm/test/test_misc/test_trip_departure_choice.py | Un-comments and updates tests with state initialization and model settings loading |
| activitysim/abm/test/test_misc/configs_test_misc/trip_scheduling_choice.yaml | Adds test configuration file with standardized field names |
| activitysim/abm/test/test_misc/configs_test_misc/trip_departure_choice.yaml | Adds test configuration file using legacy field names |
| activitysim/abm/test/test_misc/configs_test_misc/settings_60_min.yaml | Adds skim settings configuration for 60-minute time periods |
| activitysim/abm/test/test_misc/configs_test_misc/network_los.yaml | Adds network level-of-service configuration |
| activitysim/abm/models/trip_scheduling_choice.py | Refactors TripSchedulingChoiceSettings to inherit from LogitComponentSettings, adds line to sort schedule alternatives |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
activitysim/abm/test/test_misc/configs_test_misc/trip_departure_choice.yaml
Outdated
Show resolved
Hide resolved
activitysim/abm/test/test_misc/configs_test_misc/trip_departure_choice.yaml
Outdated
Show resolved
Hide resolved
| A pytest fixture that returns the data folder location. | ||
| :return: folder location for any necessary data to initialize the tests |
Copilot
AI
Oct 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring incorrectly states that this fixture 'returns the data folder location', but it actually returns the module name string 'summarize'. The docstring should be updated to accurately describe what the fixture returns and its purpose.
| A pytest fixture that returns the data folder location. | |
| :return: folder location for any necessary data to initialize the tests | |
| A pytest fixture that returns the module name string used in the test setup. | |
| :return: module name string ("summarize") |
jpn--
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some changes...
activitysim/abm/test/test_misc/configs_test_misc/trip_departure_choice.yaml
Outdated
Show resolved
Hide resolved
activitysim/abm/test/test_misc/configs_test_misc/trip_departure_choice.yaml
Outdated
Show resolved
Hide resolved
activitysim/abm/test/test_misc/configs_test_misc/trip_scheduling_choice.yaml
Outdated
Show resolved
Hide resolved
* improved testing * blacken * stable sorting * blacken
Fix for #891
This pull request introduces several improvements and refactors to the trip scheduling and departure choice components, settings, and configuration files in the ActivitySim ABM prototype and test suite. The main changes include updating configuration schemas for consistency, refactoring settings classes for better inheritance and maintainability, improving sorting and spec reading logic, and enhancing test coverage and utility functions.
Configuration and Settings Refactoring
SPECinstead ofSPECIFICATION, standardized preprocessor settings, and addedcompute_settingsandprotect_columnsfor consistency and clarity. (activitysim/examples/prototype_arc/configs/trip_scheduling_choice.yaml,activitysim/abm/test/test_misc/configs_test_misc/trip_scheduling_choice.yaml,activitysim/abm/test/test_misc/configs_test_misc/trip_departure_choice.yaml, [1] [2] [3]TripSchedulingChoiceSettingsclass to inherit fromLogitComponentSettingsand removed redundant fields, streamlining the settings schema for trip scheduling choice. (activitysim/abm/models/trip_scheduling_choice.py, activitysim/abm/models/trip_scheduling_choice.pyL347-R352)Logic and Code Improvements
tour_idandSCHEDULE_IDin the schedule generation function to ensure deterministic and organized output. (activitysim/abm/models/trip_scheduling_choice.py, activitysim/abm/models/trip_scheduling_choice.pyR85) (This was the actual fix to Reproducible random does not work in trip scheduling choice #891)SPECIFICATIONtoSPECin the segment spec reading function for consistency with configuration changes. (activitysim/abm/models/trip_scheduling_choice.py, activitysim/abm/models/trip_scheduling_choice.pyL210-R212)Testing Enhancements
activitysim/abm/test/test_misc/test_trip_departure_choice.py, activitysim/abm/test/test_misc/test_trip_departure_choice.pyL1-R177)Prototype and Test Configuration Updates
activitysim/abm/test/test_misc/configs_test_misc/network_los.yaml, [1];activitysim/abm/test/test_misc/configs_test_misc/settings_60_min.yaml, [2]activitysim/examples/prototype_arc/configs/settings.yaml, [1] [2]Miscellaneous
.gitignoreentries in test output directories for cleaner repository management. (activitysim/examples/prototype_arc/test/output/.gitignore, [1];activitysim/examples/prototype_arc/test/output/cache/.gitignore, [2];activitysim/examples/prototype_arc/test/output/trace/.gitignore, [3]