post EET merge clean up - #1098
Conversation
There was a problem hiding this comment.
Pull request overview
Updates ActivitySim’s documentation and code to (1) render math in the docs, (2) harden Poisson sampling’s empty-draw fallback behavior, and (3) standardize terminology by renaming the probability-based sampling method to inverse_cdf.
Changes:
- Enable MyST dollar-math support for Sphinx docs and update docs text accordingly.
- Fix Poisson sampling fallback to exclude unavailable (zero-probability) alternatives; add a focused unit test.
- Rename
monte_carlosampling method toinverse_cdfacross docs, configuration typing, and tests.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/users-guide/ways_to_run.rst | Updates simulation-method terminology in the user guide. |
| docs/users-guide/sampling-methods.rst | Renames documented sampling method default/option to inverse_cdf. |
| docs/dev-guide/sampling-methods.md | Updates dev docs to match inverse_cdf naming and clarifies Poisson fallback behavior. |
| docs/dev-guide/explicit-error-terms.md | Updates EET documentation terminology and math formatting. |
| docs/conf.py | Enables MyST dollarmath extension for $...$ / $$...$$ rendering. |
| activitysim/core/test/test_logit.py | Updates test comments/terminology to inverse-CDF. |
| activitysim/core/test/test_interaction_sample.py | Adds/updates tests for Poisson fallback behavior and renamed method key. |
| activitysim/core/logit.py | Adds a guard against unsupported nested-logit sampling inputs. |
| activitysim/core/interaction_simulate.py | Removes outdated “Monte Carlo fashion” phrasing from docstrings. |
| activitysim/core/interaction_sample.py | Renames sampling method literal/defaults; fixes Poisson fallback padding with unavailable alts. |
| activitysim/core/interaction_sample_simulate.py | Removes outdated “Monte Carlo fashion” phrasing and expands EET-alignment commentary. |
| activitysim/core/configuration/top.py | Updates configuration typing/docs for sample_method rename. |
| activitysim/core/configuration/base.py | Updates compute-settings typing/docs for sample_method rename. |
| activitysim/abm/models/util/bias_logsums.py | Updates warning text to refer to inverse_cdf instead of monte_carlo. |
| activitysim/abm/models/trip_scheduling_choice.py | Clarifies EET alignment behavior and limitations in trip scheduling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@janzill I agree no deprecation warnings are needed here. The two copilot review comments are both minor but I think reasonable. I tried to simply accept them so as to merge but I think I lack write access to the |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Agreed, they are reasonable so I accepted both, thanks Jeff |
Several small fixes and clean ups:
@jpn-- regarding renaming a settings parameter I assumed that it's ok without deprecation in this case because the parameter was introduced less than 24h ago and there hasn't been a release yet, but let me know if you think otherwise.