ActivitySim / activitysim

An Open Platform for Activity-Based Travel Modeling
https://activitysim.github.io
BSD 3-Clause "New" or "Revised" License
189 stars 96 forks source link

accessibility task #100

Closed bstabler closed 7 years ago

bstabler commented 8 years ago

I want to make sure that we start with the right version of MTC's TM1 accessibility script. Please confirm that we need to re-implement this script into ActivitySim as a new fully featured sub-model.

https://github.com/MetropolitanTransportationCommission/travel-model-one/blob/master/model-files/scripts/skims/Accessibility.job

DavidOry commented 8 years ago

I read the scope as translating this script as part 1 and computing destination choice logsums as part 2. I am personally far more interested in part 2 than part 1.

bstabler commented 8 years ago

Yes, the first part is to translate this script, thanks for confirming. I think it makes sense to do the second part after (or at the same time as) task 8. Ok?

DavidOry commented 8 years ago

I agree.

toliwaga commented 8 years ago

I have the translated script implemented and am getting very close but not exactly duplicate, results in python as the accessibility table supposedly created by mtc_accessibility.job.

I did notice one thing in the original job script:

In mtc_accessibility.job at line 191 we have:

     ; compute the decay function for peak transit accessibility if a round trip path is available (zero otherwise)
     if(trPkTime_od > 0 && trPkTime_do > 0)

        trPkRetail = retailEmp * exp(_kTran * trPkTime)
        trPkTotal  = totalEmp  * exp(_kTran * trPkTime)

     endif

But in mtc_accessibility.job at line 210 we have:

     ;   - off-peak, round-trip time
     trOpTime = trOpTime_od + trOpTime_do

     ; compute the decay function for off-peak transit accessibility if a round trip path is available (zero otherwise)
     if(trOpTime>0)

        trOpRetail = retailEmp * exp(_kTran * trOpTime)
        trOpTotal  = totalEmp  * exp(_kTran * trOpTime)

     endif

I am thinking we should have instead:

if(trOpTime_od > 0 && trOpTime_do > 0)

Line 214 assumes that it will always be possible to travel in both directions between any two skims at midday.

This is not the case. There are 28006 OD taz pairs for which one of these is 0 and the other is not.

orig=6, dest=362, for instance:

INFO - activitysim.defaults.models.accessibility - trOpTime_od[6, 362] = 224.6554
INFO - activitysim.defaults.models.accessibility - trOpTime_do[6, 362] = 0.0
bstabler commented 8 years ago

this looks like a bug to me so let's fix it in our version

toliwaga commented 8 years ago

Accessibility method now working with UEC-style expression file. I am getting pretty similar results to the accessibilities that were calculated by the job, and exactly the same results of my previous numpy matrix implementation.

The fact that I get exactly the same results as the job version for non motorized which makes me think maybe we have slightly different versions of the transit and traffic skims…

Now that this is working, we need to decide a few things:

toliwaga commented 8 years ago

I am submitting a pull request for the current accessibility branch, which I believe fulfills aprt one of this task: it implements the translation of MTC's TM1 accessibility script as expression files that can easily be extended or modified without updating python code. This is implemented as a sub-model that is run before other models.

Currently it does not override the existing accessibility table values, so results can be compared to the existing values. Eventually we will presumably want to do so and update the regression tests accordingly.

bstabler commented 8 years ago

go back and update the expression files

accept

yes, update all the regression tests

yes

change to skim_od and skim_do. And change all expression files to use something smarter like skim_od and skim_do as well. See #105

we need to do this as well

toliwaga commented 8 years ago

The six bullet items in the comment above are done and pushed to branch. I will pull them to master branch and close this issue - we should create a separate issue for logsum choice (part 2) as we won't be doing it until Task 8.

bstabler commented 7 years ago

While writing the accessibility module documentation, I noticed a few things to revise:

  1. We should trace the temporary expressions as well, such as _auPkTime
  2. We don't need to trace accessibility.full.csv since this goes into the data store and we don't care about the differences w.r.t. the old/original accessibilities.
  3. I don't understand the accessibility.csv trace file since it has two columns - one for the origin zone and one for the destination zone? I was only expecting to see accessibility.result.csv, which has one column - one for the OD pair. I think we can drop this output.
toliwaga commented 7 years ago

I fixed the trace files in nested-logit branch.

Will trace temp expressions soon...

toliwaga commented 7 years ago

Added tracing of temps in pull request #109