Park-and-ride lot choice and capacity#1001
Conversation
Required to maintain backwards compatibility in tour indexes in SANDAG models
|
@copilot resolve the merge conflicts in this pull request |
jpn--
left a comment
jpn--
left a comment
There was a problem hiding this comment.
Mostly good. That several agencies have collaborated and tested this already gives some extra confidence. Only a couple minor bugs/concerns to address.
| persons = state.get_dataframe("persons") | ||
|
|
||
| # don't know a-priori how many park-and-ride tours there are at the start of the model run | ||
| # giving the buffer a size equal to the number of persons should be sufficient |
There was a problem hiding this comment.
This is probably fine most of the time, but in the outside chance that the number of PnR tours does exceed the number of persons, the model is going to crash hard later, probably just after line 159 where pad will become negative.
| synced_choices = synced_choices.sort_index() | ||
|
|
||
| # now append any additional rows need to get size back to original length | ||
| pad = len(self.shared_pnr_choice) - len(synced_choices) |
There was a problem hiding this comment.
We should intercept here if pad is negative, maybe crash gracefully explaining that the number of PnR tours was unexpectedly greater than the number of persons, instead of crashing with an arcane "negative dimensions are not allowed" error from deep in this code. (see initialization comment below)
| .astype(int) | ||
| ) | ||
|
|
||
| elif self.model_settings.RESAMPLE_STRATEGY == "random": |
There was a problem hiding this comment.
There's no testing of RESAMPLE_STRATEGY == "random"
|
|
||
| # count the total number of pnr choices being resimulated | ||
| pnr_counts = ( | ||
| current_sample.pnr_zone_id.value_counts() |
There was a problem hiding this comment.
I tried making a simple test for "random" but it failed here, current_sample seems to be a pd.Series.
The correct code might be:
pnr_counts = (
tours_in_cap_zones.loc[current_sample.index, "pnr_zone_id"]
.value_counts()
.reindex(self.shared_pnr_occupancy_df.index)
.fillna(0)
.astype(int)
)
| ---------- | ||
| choices : pandas.Series | ||
| zone id of location choice indexed by person_id | ||
| segment_ids : pandas.Series |
There was a problem hiding this comment.
This function does not take segment_ids
3 participants
Code changes for #965