Revised shadow pricing mechanism #613
Conversation
…tzdy/activitysim into develop_RevisedShadowPricing
|
This PR doesn't include results from running a test of shadow pricing, because shadow pricing is still "off" in the test configuration here. Once you enable the test, you will also need to update the test's expected output. (If you enable shadow pricing but the tests still pass with the same outcomes as before, that's a problem, but I've just run the test locally with shadow pricing engaged and I do see changes in work location zones. |
| sprice.replace([np.inf, -np.inf], 0, inplace=True) | ||
|
|
||
| # shadow prices are set to -999 if overassigned or 0 if the zone still has room for this segment | ||
| self.shadow_prices[segment] = np.where( |
There was a problem hiding this comment.
When I run the 25-zone shadow pricing example with the "full" test population, which is 5000 households, the model is crashing on school location iteration 2. It seems that there are only 2 high school zones possible, and after the first iteration one is over subscribed and one is under subscribed (obviously), but it's within the percent_tolerance and both zones get closed at this line of code. The problem is that one high schooler still isn't assigned a school location, and with all zones closed the model then subsequently crashes. This is a corner case but we don't want corner cases to crash the model, and while unlikely in realistic size applications it's still conceivable.
A proposed fix: add a check here to make sure that we never close all the zones. You can check if the max sprice is less than 1 + percent_tolerance / 100 and if so, make the shadow prices such that at least that max price zone stays open. You'll probably also want to flag this condition and then prevent additional iterations / reassignments in this segment, as you're going to be as converged as you can possibly get within the tolerances of the integer simulation.
There was a problem hiding this comment.
Good catch. I didn't see it because I was running the "full" test population with multi-processing and it didn't fail. Obviously this is bad because it means single process and multi-process results were different! The problem stemmed from the sampling of people from over-subscribed zones being done with the built in pandas sample. The sampling logic was re-written to use ActivitySim's RNG. I have turned on shadow pricing for the small test example (looks like shadow pricing was never included in any CI tests before this!) and included a multi-process test as well.
As for the edge case you found. Totally agree that we don't want this to crash the model. We had some logic in location_choice.py that was trying to deal with this instance where there were no available zones, but it didn't catch the edge case where one segment was converged but other segments were not (e.g. high school converged but not college). The implemented solution was to treat the segment as converged if all of the zones are within tolerance and then then not sample any people for re-simulation. By dealing with this fix in the sampling step, we eliminated the need for the logic in the location_choice.py to try to catch unavailable zones.
All of these changes have been pushed and the (updated) tests are passing.
There was a problem hiding this comment.
Thanks @dhensle! I confirm my edge case looks fixed and things are running more smoothly now.
simulation-based shadow pricing method implementation, with more details on the method discussed here.