search

BayAreaMetro / bayarea_urbansim

Bay Area Version of the UrbanSim Model
http://bayareametro.github.io/bayarea_urbansim
13 stars 11 forks source link

Cleaning up BAUS - Feb 2020 #120

Open smmaurer opened 4 years ago

smmaurer commented 4 years ago

This issue contains running documentation of work to clean up Bay Area UrbanSim and merge outstanding pull requests. cc @mkreilly

General goals:

Confirm that current codebase runs

The first thing I've done is check that the current codebase runs.

Codebase: BayAreaMetro/bayarea_urbansim master, at commit 423bb5b Data files: "Current Large General Input Data", last updated 2019-07-11 Operating system: MacOS 10.15 Catalina

python baus.py -c -y 2015 -s 4 --random-seed >> runs/log10.txt 2>&1

This environment is working:

That Pandana version is key. Bay Area UrbanSim does NOT run for me with the latest version of Pandana (0.4.4). Here's the error. Looks like probably a difference in how missing values are handled.

And I get the same error with the PR #117 branch running in Python 3, so it seems like not something that's been fixed elsewhere yet.

mkreilly commented 4 years ago

@fscottfoti Sam is doing some cleanup here. Please tell us if there is anything we should know about pandana (above) if you have the time. Thanks!

fscottfoti commented 4 years ago

Honestly it looks like something in kdtree itself (from sklearn) is breaking. Probably that version of Pandana has a new version of kdtree with some change that broke things? Just a guess. Also worth checking to make sure the x's and y's on the parcels are still all valid (and probably should be UTM).

fscottfoti commented 4 years ago

Just from looking at the error and some of the kdtree code, I would suspect that one of the x's or y's is nan. If the older versions of pandana let this pass, maybe kdtree changed the behavior to do this validation to raise an error on nan but didn't before. Not at all sure, but it's a theory.

smmaurer commented 4 years ago

Excellent. Thanks @fscottfoti, I'll check these things out!

smmaurer commented 4 years ago

The outstanding PR's are merged now, and I'm working through various minor errors. Here are some items to come back to:

Pandana

The Pandana problem indeed stemmed from parcel records without x-y coordinates. These used to be filtered out automatically when parcels were matched to network nodes, but no longer are after some changes in scikit-learn.

Python 3

One problem I'm seeing in Python 3 is that urbansim has some incompatibilities with Pandas v1.0, which was released about a month ago. For now, we should just pin BAUS at Pandas v0.25. An upcoming release of urbansim should fix things: https://github.com/UDST/urbansim/pull/222

Miscellaneous

Also tagging @yuqiww, who i think will be interested in this thread.

fscottfoti commented 4 years ago

That seems like a reasonable set of items to me! I would think BAUS should not have any parcels with no x/y so that should probably be filtered in BAUS and then the pandana issue is not important from the perspective of BAUS. It would be nice to use pandas 1.0 so it'll be good to unpin when UrbanSim supports that.

On Fri, Mar 20, 2020 at 1:40 PM Sam Maurer notifications@github.com wrote:

The outstanding PR's are merged now, and I'm working through various minor errors. Here are some items to come back to: Pandana

The Pandana problem indeed stemmed from parcel records without x-y coordinates. These used to be filtered out automatically when parcels were matched to network nodes, but no longer are after some changes in scikit-learn.

  • open a separate issue about fixing our base data
  • open an issue in udst/pandana to discuss restoring the previous default behavior

Python 3

One problem I'm seeing in Python 3 is that urbansim has some incompatibilities with Pandas v1.0, which was released about a month ago. For now, we should just pin BAUS at Pandas v0.25. An upcoming release of urbansim should fix things: UDST/urbansim#222 https://github.com/UDST/urbansim/pull/222

  • Revisit Pandas version restriction after urbansim is updated

Miscellaneous

  • Update dependency lists and installation instructions

Also tagging @yuqiww https://github.com/yuqiww, who i think will be interested in this thread.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/BayAreaMetro/bayarea_urbansim/issues/120#issuecomment-601901302, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOHNIGSCGKCIBAVAACKVZLRIPIEZANCNFSM4KPJHYCA .