search

ual / bayarea_urbansim

UAL research fork of Bay Area UrbanSim
2 stars 1 forks source link

ual.py household reconciliations #9

Open mxndrwgrdnr opened 7 years ago

mxndrwgrdnr commented 7 years ago

I think there is a bug around line 367 of ual.py. Line 358 seems to drop the household_id from the households table with this command:

hh = households.to_frame(['unit_id', 'building_id']).reset_index()

and then on line 367 it attempts to merge with the units table and use household_id as the index only that column is no longer associated with the households table. can you confirm, Sam? Looks like this is the result of changes that were made in June.

smmaurer commented 7 years ago

I'm not sure this is an issue. I haven't verified it, but my reading of the code is that 'households' has an index named 'household_id' (per orca_test assertion), and reset_index() demotes it to a column with the same name. So it should still be there when it's re-designated as the index of the merged table.

https://github.com/ual/bayarea_urbansim/blob/ual-development/baus/ual.py#L360-L368

From your email, this came up because an orca_test assertion involving that index is failing somewhere, right? My first thought is that it could be a bug in orca_test, actually. Pandas treats indexes as completely separate from the rest of the DataFrame, which makes those bits of orca_test more bug prone because there are multiple cases to cover.

In December we merged an orca_test PR that added support for multi-indexes.. Maybe this broke support somewhere for DataFrames with just a single index?

https://github.com/UDST/orca_test/pull/13/files

mxndrwgrdnr commented 7 years ago

I had this thought as well. I commented out the orca_test check but the code just fails later when it tries to set household_id as the index at line 368, saying there is no column named household_id even after the reset_index() is called which should set the index to a column. I just tried debugging by messing with some of the orca_test warnings and it looks like the index isn't named. Trying to figure out at what point that might be happening, or if I can work around it by defining the index name.

mxndrwgrdnr commented 7 years ago

OK I just pushed these changes to ual-development and it seems to have solved the issue. It's kind of a hack but it works for now.

I had to comment out the orca_test functionality bc for some reason orca isn't storing the index name in the households table.