Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Time of Day Hard Coding #92

Open
bettinardi opened this issue Feb 25, 2020 · 5 comments
Open

Time of Day Hard Coding #92

bettinardi opened this issue Feb 25, 2020 · 5 comments

Comments

@bettinardi
Copy link
Collaborator

It has been identified that the 5 time periods are hard coded in SOABM.py - https://github.com/RSGInc/SOABM/blob/master/template/scripts/SOABM.py#L1073

The user inputs the time of day specification in at least two spots:

  1. https://github.com/RSGInc/SOABM/blob/master/template/config/cvm/TOD_Periods.csv
  2. https://github.com/RSGInc/SOABM/blob/master/template/config/soabm-skims.xlsx

For this issue we should consider building the cvm/external model TOD information from soabm-skims.xlsx, so that the input is in one location. But at a minimum, SOABM.py should be updated to use the Time of day information from one of the input files as opposed to being hard coded.

@bettinardi
Copy link
Collaborator Author

It has also been noted that it appears that the 0 position for EV1 in SOABM.py is being incorrectly assigned to EV (which is supposed to only go to 3am): https://github.com/RSGInc/SOABM/blob/master/template/scripts/SOABM.py#L1073

Note the agreed upon definitions of the ABM time periods 1-40 are captured here: https://github.com/RSGInc/SOABM/wiki/Model-Outputs#time-period-codes

So when the time of day matrices are being built EV is incorrectly getting time of day period 1 (python position 0) associated to it, instead of the EA period (which is where 3-5 am should go).

This should all be cleaned up, when SOABM is properly linked to Time of Day input files as opposed to having hard coded references in SOABM.py. So when SOABM.py is updated it needs to properly build the set of 1-40 ABM time periods that are associated with each of the input time of day periods.

@bettinardi bettinardi added the bug label Feb 28, 2020
@bettinardi
Copy link
Collaborator Author

Capturing detailed explanation from Binny:

The visualizer table is correct. That is how we code the surveys and process CTRAMP outputs. The first tod bin is 3-5am, then its half hour bins (2 to 39) from 5 am to midnight. Bin 40 covers midnight-3 am.

Now, CTRAMP does not output a zero tod_bin value. They strictly go from 1 to 40. That is why [0,0] does not represent any time period in a day. Let’s look at the function that returns the aggregate time period index for a given tod40_bin.

def whichTimePeriod(deptTime, timePeriodStarts):

return(len(timePeriodStarts[deptTime >= timePeriodStarts])-1)
The above function returns the index of the aggregate time periods for assignment going from 0 to 5.

timePeriods = ["EV1","EA","AM","MD","PM","EV2"]
timePeriodStarts = [0 ,1 ,7 ,10 ,26 ,30 ]

Example 1: tod40_bin = 1

The expression len(timePeriodStarts[1 >= timePeriodStarts]) counts the number of values in timePeriodStarts that are greater than or equal to “1”. Both 0 and 1 are >= 1, so the expression is evaluated as:
len(timePeriodStarts[1 >= [0 ,1 ,7 ,10 ,26 ,30 ]])-1 = (2 – 1) = 1, which is EA

Example 2: tod40_bin = 25
len(timePeriodStarts[25 >= [0 ,1 ,7 ,10 ,26 ,30 ]])-1 = 4 - 1 = 3, which is MD

Example 3: tod40_bin = 26
len(timePeriodStarts[26 >= [0 ,1 ,7 ,10 ,26 ,30 ]])-1 = 5 - 1 = 4, which is PM

So, you can see that 25 falls in MD, while 26 falls in PM. Therefore, 26 (5-5:30 pm) marks the beginning of the PM period.

Now, why do we have EV1 for resident matrices? This is for easy implementation of loops when loading the matrices for assignment. Remember the truck models do have EV1 which is summed together with EV2 for assignment. To process the resident matrices as well in the same loop, the build_trip_matrices procedure produces EV1 trip matrix for resident demand as well but with all zeroes. So, removing EV1 would require additional code changes and testing.

@bettinardi
Copy link
Collaborator Author

Response from Alex:
So the correction is to change the time period starts vector from:
timePeriodStarts = [0 ,1 ,7 ,10 ,26 ,30 ]

To:
timePeriodStarts = [0 ,1 ,6 ,9 ,25 ,29 ]

Is that correct? That’s all I need to do to correct the code?

@binnympaul
Copy link
Contributor

Yes, that should fix it.

@bettinardi bettinardi removed the bug label Apr 10, 2020
@bettinardi
Copy link
Collaborator Author

The bug was addressed in early March. Keeping this issue, because initial statement is still true. Time of day coding is occurring in too many places in the inputs / code. So the issue stands until time of day coding is resolved and better captured in the instructions. All of the know bugs / errors have been resolved.

@bettinardi bettinardi added this to the Mid-Level Priority milestone Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants