[i-am-sijia]

Addressing #1082 with option 2 ...

Summary generated by Copilot (reviewed):

This pull request refactors how chooser segments are handled in the shadow pricing logic, making the configuration more flexible and robust. It uses CHOOSER_SEGMENT_COLUMN_NAME from the location choice model setting for chooser segmentation to keep shadow-pricing resampling consistent with segment_ids in location choice settings, adds better error handling for misconfigurations, and introduces new tests to cover these scenarios.

Shadow Pricing Configuration Improvements:

• Removed the hardcoded SEGMENT_TO_NAME mapping from both the module-level dictionary and the ShadowPriceSettings class.
• The ShadowPriceCalculator now uses the CHOOSER_SEGMENT_COLUMN_NAME from location choice model settings to determine the chooser segment column dynamically.

Error Handling Enhancements:

• In update_shadow_prices, added explicit checks to raise a SystemConfigurationError if the configured chooser segment column is missing from persons_merged, or if there is no overlap between SEGMENT_IDS values and the values present in the chooser segment column.

Testing Improvements:

• Added new unit tests to verify that shadow pricing works with customized chooser segments (e.g., a string column) and that misconfigurations (such as non-overlapping segment values) are properly detected and raise clear errors.
• Updated test fixtures to include string-valued chooser segment columns for more comprehensive coverage.

[jpn--]

As discussed in engineering 8/27, this needs a unit test. @i-am-sijia will tackle this.

[Copilot]

🟡 Changes recommended

Validation includes ineligible persons, and removing a previously supported setting breaks existing configurations.

2 open findings

• Preserve deprecated SEGMENT_TO_NAME configuration compatibility · New
• Validate segment overlap only for modeled choosers · New

What changed in this PR

Updates simulation-based shadow pricing to use each location model’s configured chooser segmentation.

Changes:

• Uses CHOOSER_SEGMENT_COLUMN_NAME for resampling.
• Adds configuration validation and custom-segment tests.

File	Description
activitysim/​abm/​tables/​shadow_pricing.py	Refactors segmentation and adds validation.
activitysim/​abm/​test/​test_misc/​test_shadow_pricing_simulate.py	Tests string segments and non-overlapping values.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[bhargavasana]

From SANDAG's perspective, I am comfortable with the proposed segmentation logic and supportive of merging this fix. We previously tested the equivalent configuration correction and documented results in SANDAG/ABM#434. The spatial fit between modeled workplace assignments and land-use employment improved, although some outliers remained. The corrected run did not meet the convergence criterion within ten iterations, so further improvement in convergence and model validation remains a separate consideration.

Could we update the shadow-pricing documentation (https://activitysim.github.io/activitysim/develop/dev-guide/components/shadow_pricing.html) as part of this PR, or create a linked documentation follow-up? I think the segmentation settings could use some clarification, particularly for agencies using simulation-based shadow pricing.

• Explain that simulation-based shadow pricing now uses CHOOSER_SEGMENT_COLUMN_NAME from the location-choice model settings to identify persons for resimulation, and that SEGMENT_TO_NAME is deprecated and ignored.
• Clarify that segmenting workplace location choice by occupation or income does not require occupation- or income-specific spatial employment controls. For example, SANDAG segments workplace choice by occupation, but all occupation segments share the same total_emp_revised target. ActivitySim combines the modeled workers across segments sharing that target when evaluating shadow-pricing fit.
• Explain that the simulation method compares modeled spatial shares against employment/enrollment target shares, rather than requiring absolute regional worker and job totals to match.