activitysim icon indicating copy to clipboard operation
activitysim copied to clipboard

size_terms.py in multizone branch expects land use index to be named 'zone_id'

Open stefancoe opened this issue 4 years ago • 2 comments

The name of the land use index column is configurable in settings.py, but there is an assert statement here https://github.com/ActivitySim/activitysim/blob/multizone/activitysim/abm/tables/size_terms.py#L90 that expects it to be called 'zone_id.'

In the existing MTC example, the zone column and index are both configured as 'TAZ': https://github.com/ActivitySim/activitysim/blob/master/activitysim/examples/example_mtc/configs/settings.yaml#L33

stefancoe avatar Dec 09 '20 01:12 stefancoe

This is a change from master. The input_table_list setting in multizone branch have been changed to conform with the new (internal) naming convention.

We could change the settings file to allow EITHER name though this risks introducing errors if 3-zone users use it as a template, since in 3-zone systems TAZ is not the right index for land_use.

  - tablename: land_use
    filename: land_use.csv
    index_col: zone_id
    rename_columns:
      # allow either ZONE or TAZ
      ZONE: zone_id
      TAZ: zone_id
      COUNTY: county_id
    keep_columns:
      - DISTRICT
      - SD
      - county_id

I think it is more straightforward to encourage explicit rename and identification of index column, as in the existing multizone branch example_mtc settings file:

  - tablename: land_use
    filename: land_use.csv
    index_col: zone_id       <- install ‘zone_id’ column as index (renamed from ‘TAZ’ to ‘zone_id’ in rename_columns settings)
    rename_columns:
      TAZ: zone_id           <- rename TAZ column from incoming csv file to ‘zone_id’
      COUNTY: county_id
    keep_columns:
      - DISTRICT
      - SD
      - county_id
      - . . .

toliwaga avatar Dec 09 '20 07:12 toliwaga

@toliwaga the last option seems good. How about also adding an additional comment to every non-configurable setting, such as:

this must be 'zone_id'; it is not configurable.

stefancoe avatar Dec 10 '20 18:12 stefancoe