Lab 583 support vehicles #58

KasiaKoz · 2021-02-24T09:34:57Z

adds vehicles and vehicle_types attribute to Schedule object.
- vehicles is a mapping between vehicle IDs stored within Routes and their types so for a bus route with a trip which uses vehicle 'veh_1', there will be an entry 'veh_1': {'type': 'bus'}. Types need not be exactly the same as mode of the route but are as default if generated by genet.
- vehicle_types is a dictionary holding vehicle definitions which used to be hard coded here. Now you can pass a vehicle definitions config when instantiating a schedule. If not passed, it defaults to that example config (so no change in behaviour, but gives flexibility to change definitions for your use case and also modify them when the Schedule already exists.
reads vehicle IDs in MATSim schedule.xml file and the vehicles.xml (which forms vehicles and vehicle_typesattributes) if passed
changes route.trips attribute to hold vehicles next to trips. Before

{'trip_1': '07:00:00', ...}

After

{'trip_id': ['trip_1', 'trip_2', ..], 'trip_departure_time': ['07:00:00', '08:00:00', ...], 'vehicle_id': ['veh_1', 'veh_2', ...]}

which also makes it look more sensible in route data summary method. From

n.schedule.route_attribute_summary(data=True)

attribute
├── route_short_name: ['N20', '134', 'N55', '98', 'N5']
├── mode: ['bus']
├── trips
│   ├── VJ2cdccea96e0e3e6a53a968bcb132941415d6d7c9_04:53:00: ['04:53:00']
│   ├── VJ375a660d47a2aa570aa20a8568012da8497ffecf_03:53:00: ['03:53:00']
│   ... massive list, all trips show up
│   └── VJf8c00ef586e4fbf1e7c75d216ed830d039b6e04a_04:23:36: ['04:23:36']

to

n.schedule.route_attribute_summary(data=True)

attribute
├── route_short_name: ['94', '113', 'N8', 'N20', 'N5']
├── mode: ['bus']
├── trips
│   ├── trip_id: ['VJ33e5546bdd3e9281dc8dbe530d67dd7e34352b61_15:09:00', 'VJ7d40809e4e187795fcb14f1befa17036baedb04c_17:32:00', 'VJ093350c7a24291d42b83678a7f298e6c6c875e9f_08:51:00', 'VJe3087fc34c73052fcfbbf9e3256adf7f629f38a9_24:01:00', 'VJ6cc11fb46ab954b63161bf1bee99b178603e3bb7_16:07:00']
│   ├── trip_departure_time: ['09:58:13', '18:06:00', '02:44:00', '09:46:34', '11:07:34']
│   └── vehicle_id: ['veh_1853_bus', 'veh_1715_bus', 'veh_765_bus', 'veh_926_bus', 'veh_529_bus']

adds a couple of cool methods
- route_trips_to_dataframe which generates a DataFrame holding all the trips, their departure times and vehicle IDs, next to the route ID and service ID. ---!! I need help with naming this method, there is another one generate_trips_dataframe which extends this further, expanding across all stops (for all trips) in the route and giving scheduled timestamps at those stops for all trips for all routes, I think they sound too similar and I'm not sure they convey what the outputs are very well >_>
- set_route_trips_dataframe which takes the output of route_trips_to_dataframe and changes route.trips attributes of all routes present in the DataFrame to what is in the passed DataFrame. This means you can pull out a DataFrame representation of all trips in the Schedule, change their times, IDs or vehicles (so maybe have some vehicles shared based on some logic) and then update the Schedule with those changes just using that DataFrame representation. Example from a jupyter notebook:

be due to missing suitable attribute change methods, which are coming next

applying functions and dictionary maps to iterator objects of the same shape

iterating more than once issue

lower level objects

fix a couple of tests

methods for network

methods

of interest

# Conflicts: # genet/core.py # tests/test_core_schedule_elements.py

or service, test the option to pass schedule.stop('id') for this purpose too

# Conflicts: # genet/exceptions.py # genet/outputs_handler/matsim_xml_writer.py # genet/schedule_elements.py # genet/utils/persistence.py # genet/variables.py # tests/test_core_components_route.py # tests/test_core_components_service.py # tests/test_core_schedule.py # tests/test_core_schedule_elements.py # tests/test_utils_persistence.py

gac55

Slick. Hard to imagine making these kind of changes without good tests now ;)

gac55 · 2021-02-25T13:07:44Z

genet/configs/vehicles/vehicle_definitions.yml

@@ -0,0 +1,155 @@
+# form basis for vehicle modes in the schedule, used when reading in GTFS and present in the schedule
+# unless replaced by reading vehicles.xml file
+VEHICLE_TYPES:


Much easier to read

And also much easier to edit! 👍

gac55 · 2021-02-25T13:09:18Z

genet/schedule_elements.py

+            # check mode consistency
+            vehicles_to_modes = df.groupby('vehicle_id').apply(lambda x: list(x['type'].unique()))
+            if (vehicles_to_modes.str.len() > 1).any():
+                # there are vehicles which are shared across routes with different modes


Does this ever happen? Would hope not but good to have a catch for it.
If it does with our ITO data it is something we should feed back to them.

so when reading GTFS I give unique vehicles to each trip so this doesn't happen, is there data in the GTFS that could serve as a vehicle? I added this because

people might like to generate vehicles that are shared between trips to make things more realistic, and it's really easily done with the route_trips_to_dataframe and set_route_trips_dataframe methods

People could get their vehicle ids muddled when adding routes and services

# Conflicts: # tests/test_core_schedule.py

mfitz

Looks good. Added some minor comments.

...route_trips_to_dataframe which generates a DataFrame holding all the trips, their departure times and vehicle IDs, next to the route ID and service ID. ---!! I need help with naming this method, there is another one generate_trips_dataframe which extends this further, expanding across all stops (for all trips) in the route and giving scheduled timestamps at those stops for all trips for all routes, I think they sound too similar and I'm not sure they convey what the outputs are very well >_>

How about route_trips_to_dataframe and route_trips_with_stops_to_dataframe? Or route_trips_to_summary_dataframe and route_trips_to__full_dataframe?

mfitz · 2021-03-02T15:20:56Z

genet/configs/vehicles/vehicle_definitions.yml

@@ -0,0 +1,155 @@
+# form basis for vehicle modes in the schedule, used when reading in GTFS and present in the schedule
+# unless replaced by reading vehicles.xml file
+VEHICLE_TYPES:


And also much easier to edit! 👍

mfitz · 2021-03-02T15:21:38Z

genet/exceptions.py

@@ -60,3 +60,10 @@ class StopIndexError(Exception):
    Raised in case of Stop indexing inconsistency
    """
    pass
+
+
+class InconsistentVehicleModeError(Exception):


brownie_points += 100

I'm hooked now

mfitz · 2021-03-02T15:27:08Z

genet/inputs_handler/matsim_reader.py

@@ -219,9 +220,15 @@ def write_transitLinesTransitRoute(transitLine, transitRoutes, transportMode):

            route = [r_val['link']['refId'] for r_val in transitRoute_val['links']]

-            trips = {}
+            trips = {
+                'trip_id': [],


Is there a one-to-one relationship between trip IDs, trip departure times, and vehicle IDs? Or will these 3 lists each be of different sizes?

yeah it's one to one, so at index x in each list there should be a time and vehicle corresponding to that trip_id. It would make sense to make a Trip class and just have a list here but the nestedness of classes is destroying me 😭 Schedule(Service(Route(Stop))) is about as much as I can handle. That's partly why I introduced the dataframe setting method. Thinking about it now, it might be good to have something like this when instantiating a Route, instead of giving the trips dictionary that is exactly matching those assumptions, you can just give a dataframe with those columns, that would enforce equal length of those lists and would be more user friendly I think

The Trip class idea was why I was asking this question. I don't think it needs to sit inside a class hierarchy though. You could use a python dataclass, maybe, if the class is really just a way to hold data and won't define any logic.

ooh, first time I'm seeing this, I like! I'll add a jira card to that effect, thanks!

mfitz · 2021-03-02T15:39:15Z

genet/inputs_handler/matsim_reader.py

+            read_capacity = False
+        elif tag == 'capacity':
+            read_capacity = True
+        elif read_capacity:


Does this belong in the elif? Wouldn't it work as just an if?

took me a while to clock this 😂 , the capacity attributes get read before the capacity tag appears, the flag read_capacity will change to True, and then the vehicle type dictionary v will get its capacity set as an empty dictionary (the value of the capacity elem.attrib)

mfitz · 2021-03-02T15:40:05Z

genet/outputs_handler/matsim_xml_values.py

@@ -98,115 +98,3 @@
        # 'freespeedFactor': 1.0,
        'capacity': 9999.0}
 }
-
-
-MODE_DICT = {


Delete is the best refactoring 😄

mfitz · 2021-03-02T17:53:43Z

tests/test_core_schedule.py

+
+def test_generating_vehicles_with_shared_vehicles_and_consistent_modes(mocker, schedule):
+    schedule.vehicles = {}
+    mocker.patch.object(DataFrame, 'drop',


I like this test because it's readily apparent from reading the test alone why we should expect the generated vehicles map to look this way.

mfitz · 2021-03-02T18:03:59Z

tests/test_core_schedule.py

+                                                  'v_3': {'type': 'rail'}})
+
+
+def test_generating_additional_vehicles_by_default(schedule):


Are we testing two things here? The test name suggests we're going to assert that an additional vehicle has been generated, but the assertion also seems to be checking that the modes have changed for the pre-existing vehicles. If so, these might be better in separate tests.

nah, it's checking that all the others haven't changed, they are all the weird _bus type

mfitz · 2021-03-02T18:06:16Z

tests/test_core_schedule.py

+                                         'veh_1_bus': {'type': 'bus'}, 'veh_2_bus': {'type': 'bus'}})
+
+
+def test_generating_vehicles_with_shared_vehicles_and_inconsistent_modes(mocker, schedule):


A better test name might be something like test_rejects_inconsistent_modes_when_generating_vehicles

Test names like does_something_when_something can give a better clue about the intent of the test and the expected behaviour of the code under test.

mfitz · 2021-03-02T18:08:48Z

tests/test_data/matsim/vehicles.xml

@@ -1,10 +1,10 @@
 <?xml version="1.0" encoding="UTF-8"?>

 <vehicleDefinitions xmlns="http://www.matsim.org/files/dtd" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.matsim.org/files/dtd http://www.matsim.org/files/dtd/vehicleDefinitions_v1.0.xsd">
-	<vehicleType id="Bus">


Ah, MATSim XML schemas and case...

mfitz · 2021-03-02T18:11:53Z

tests/test_utils_persistence.py

@@ -33,6 +33,19 @@ def test_swallows_exceptions_making_new_directories(mocker):
    os.makedirs.assert_called_once()


+def test_is_yml_identifies_yml():
+    zip_dir = os.path.join('path', 'to', 'dir', 'file.yml')


zip_dir feels like a misleading name. Although you don't really need the variable:

assert persistence.is_yml(os.path.join('path', 'to', 'dir', 'file.yml'))

copy pasta fail, typical >_>'

KasiaKoz added 30 commits February 2, 2021 13:40

tidy up clashing tests

2c6748d

pass mode to schedule as is

e8dfba2

initial changes to underlying schedule structure for wider graph sharing

5fb248e

unify schedule elements data structures, failing tests are believed to

b96582e

be due to missing suitable attribute change methods, which are coming next

get rid of copy methods

1771744

add methods to build dataframes from (id, data) iterator and

9d77efa

applying functions and dictionary maps to iterator objects of the same shape

replace old df building methohds from attributes in network, fix

0cb0e57

iterating more than once issue

ensure unique indexing, add data access and modification methods

a546c57

update simplification with new schedule modification methods

5a397be

generate trips dataframe on schedule level without reinstantiating

1c53209

lower level objects

remove uniquely indexed checks, all is forced to be uniquely indexed now

f79fc84

fix a couple of tests

generalise extracting ids satisfying attribute conditions, add specific

a20251f

methods for network

add methods to extract services, routes and stops on conditions

04eae89

add extracting stops, routes, services on modal condition convenience

54e4445

methods

add spatial utils and convenience methods

830191b

add spatial and modal convenience methods for extracting links/nodes

7542a46

of interest

add forgotten not implemented error

c9154bf

add adding methods to schedule

b6d9a75

add remove methods for schedule

665e218

clean up changelog updates

e7adc50

fix test

e57289f

create a shared changelog to enable reindexing modification events

2f395a3

fix flake8

aad4b06

update notebooks

1c818fd

Merge branch 'master' into update-schedule-structure

cd88f5f

# Conflicts: # genet/core.py # tests/test_core_schedule_elements.py

instantiate route and service objects with additional attributes

fe2a7ed

add option to add routes with stop ids only

9787cf7

add data comparison for stops in schedule and to be added with route

bf00749

or service, test the option to pass schedule.stop('id') for this purpose too

fix flake8

1782062

update notebook

1652c36

KasiaKoz added 14 commits February 19, 2021 20:03

fix tests

2a878c1

update doc strings

020eb6e

add warnings

b1936e1

fix flake8

99dc7b0

vehicle types no longer a global variable

9648330

add read vehicles xml method

f2ebf53

add convenience method to change/set trips details

ba1d0e0

doc string and refine the test

fb37bdc

fix up vehicle configs read

2d2def9

'fix' flake8

2a988cf

update vehicle generation method

8715a9e

add example vehicles and update notebooks

d58c900

KasiaKoz requested review from mfitz and gac55 February 24, 2021 09:34

add vehicles inputs to pre baked scripts

a5599d5

gac55 approved these changes Feb 25, 2021

View reviewed changes

KasiaKoz added 4 commits March 2, 2021 11:20

enable generation of vehicles from scratch

063d7e0

Merge branch 'master' into LAB-583-support-vehicles

5aaf687

# Conflicts: # tests/test_core_schedule.py

fix tests post master merge

c3dd504

Merge branch 'master' into LAB-583-support-vehicles

68fe517

mfitz approved these changes Mar 2, 2021

View reviewed changes

KasiaKoz added 4 commits March 3, 2021 11:45

initial PR comments changes

e654d17

Merge branch 'master' into LAB-583-support-vehicles

a227fd8

update notebooks

297076a

fix tests

4090eed

KasiaKoz merged commit b625164 into master Mar 15, 2021

KasiaKoz deleted the LAB-583-support-vehicles branch March 15, 2021 11:42

KasiaKoz mentioned this pull request Apr 1, 2022

Losing vehicle ids #18

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lab 583 support vehicles #58

Lab 583 support vehicles #58

KasiaKoz commented Feb 24, 2021

gac55 left a comment

gac55 Feb 25, 2021

mfitz Mar 2, 2021

gac55 Feb 25, 2021

KasiaKoz Feb 25, 2021

gac55 Feb 25, 2021

mfitz left a comment

mfitz Mar 2, 2021

mfitz Mar 2, 2021

KasiaKoz Mar 3, 2021

mfitz Mar 2, 2021

KasiaKoz Mar 3, 2021

mfitz Mar 3, 2021

KasiaKoz Mar 3, 2021

mfitz Mar 2, 2021

KasiaKoz Mar 3, 2021

mfitz Mar 2, 2021

mfitz Mar 2, 2021

mfitz Mar 2, 2021

KasiaKoz Mar 3, 2021

mfitz Mar 2, 2021

mfitz Mar 2, 2021

mfitz Mar 2, 2021

KasiaKoz Mar 3, 2021

		'v_3': {'type': 'rail'}})


		def test_generating_additional_vehicles_by_default(schedule):

		'veh_1_bus': {'type': 'bus'}, 'veh_2_bus': {'type': 'bus'}})


		def test_generating_vehicles_with_shared_vehicles_and_inconsistent_modes(mocker, schedule):

Lab 583 support vehicles #58

Lab 583 support vehicles #58

Conversation

KasiaKoz commented Feb 24, 2021

gac55 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mfitz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment