search

psrc / urbansim2

3 stars 0 forks source link

Some people from households that move should be disconnected from their jobs #128

Open stefancoe opened 6 years ago

stefancoe commented 6 years ago

In our current urbansim implementation, I believe this happens probabilistically using the distance the household moves. Re-implement for urbansim2.

hanase commented 6 years ago

Currently we break connections to jobs for those workers for which the new distance to work (max within household) is larger than the previous distance. It's deterministic.

The variable should be similar to the avg_network_distance_from_home_to_work, taking maximum instead of average. That one though is an interaction variable with a set of possible location zone, whereas here we just need a household variable with one location zone per household.

Or, if it's easier, we could skip the maximum over households and simply disconnect workers with larger network_distance_from_home_to_work after moving.

stefancoe commented 6 years ago

@hanase The code for this starts on line 67 of models.py

This can be closed once reviewed.

hanase commented 6 years ago

@stefancoe It looks good. Few comments:

  1. The code block should be made optional. We could put a switch for this into settings.yaml (under household_location_choice_model it could be called say remove_jobs_from_workers, with True as default).
  2. A more generic comment: the use of .to_frame() should be limited. What it does is that it computes ALL variables on the dataset defined in the various variables files. That can slow things down a lot and wastes memory. A better way is to direct the to_frame() function to include only local attributes (i.e. not computed) and variables that are needed for the specific model. Here is an example. (There are more offenders of this rule in the original code, e.g. in the relocations models.)
  3. Again, more generic: settings.yaml could also have a node model_clear_cache which models would read and clean after themselves if it is True.

Otherwise it's very nice - thanks Stefan!

stefancoe commented 6 years ago

Thanks @hanase . Yeah, I have been meaning to go back and make any usage of .to_frame() more selective in the columns that are included in the resulting dataframe. I'll make an issue about it.