Improving Traffic Light Tutorial #642
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Revised Section 2 of the tutorial, and removed the incorrect description of the traffic light phase
…hkan-development
Pull Request Test Coverage Report for Build 4257
💛 - Coveralls |
…flow into ashkan-development
|
This is ready to review and possibly merge @eugenevinitsky |
| @@ -102,9 +108,15 @@ | |||
| "## 2. Defining Traffic Light Phases \n", | |||
| "\n", | |||
| "\n", | |||
| "To start off, we define how Sumo represents traffic light phases. A phase is defined as the states that the traffic lights around an intersection can take. The typical four-way, traffic-light-controlled intersection is modeled by a string of length 12. Consider the phase \"GrGr\". Every letter in this phase string (\"G\", \"r\", \"G\", \"r\") corresponds to an edge in the intersection, in clockwise order. Explicitly, the northern and southern edges of the intersection both have a state of \"G\", where the eastern and western edges of the intersection both have a state of \"r\". \n", | |||
| "To start off, we define how SUMO represents traffic light phases. A phase is defined as the states that the traffic lights around an intersection can take. The phase of a typical four-way, traffic-light-controlled intersection is modeled by a string (of length 4, 8, or 12, etc., depending on the type of the signal). \n", | |||
There was a problem hiding this comment.
'depending on the structure of the intersection' not the type of the signal
| "Once the TrafficLightParams class is instantiated, traffic lights can be added via the classes' `add` function. One prerequisite of using this function is knowing the node id of any node you intend to manipulate. This information is baked into the experiment's scenario class, as well as the experiment's nod.xml file. For the experiment we are using with 2 rows and 3 columns, there are 6 nodes: \"center0\" to \"center5\".\n", | ||
| "\n", | ||
| "In this particular example, each of the 6 traffic light nodes corresponds to the same set of possible phases; in other words, at any time, each node will be at the same phase. You can, however, customize each traffic light node to have different phases." | ||
| "Once the `TrafficLightParams` class is instantiated, traffic lights can be added via the `add` function. One prerequisite of using this function is knowing the node id of any node you intend to manipulate. This information is baked into the experiment's scenario class, as well as the experiment's `nod.xml` file. For the experiment we are using with 2 rows and 3 columns, there are 6 nodes: \"center0\" to \"center5\"." |
There was a problem hiding this comment.
Also, can you describe the relative ordering of the nodes e.g. center0 is the bottom left, center2 is the bottom right, etc?
There was a problem hiding this comment.
I think it's actually nod.xml
| ] | ||
| }, | ||
| { | ||
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "Following this step, the TrafficLightParams class should be passed into your scenario as element `traffic_lights`." | ||
| "In this particular example, each of the 6 intersections corresponds to the same set of possible phases; in other words, at any time, all intersections will be at the same phase. You can, however, customize each traffic light node to have different phases." |
There was a problem hiding this comment.
This is a tiny bit hard to parse. I think it is important to make clearer that this is only true for this specific example, and not for the grid in general.
| @@ -266,24 +240,24 @@ | |||
| "source": [ | |||
| "For more flexibility than the static traffic lights defined above, and more control than RL-controlled traffic lights, actuated traffic lights are a good option to consider.\n", | |||
| "\n", | |||
| "As an excerpt from Sumo's documentation: \"SUMO supports gap-based actuated traffic control This control scheme is common in Germany and works by prolonging traffic phases whenever a continuous stream of traffic is detected. It switches to the next phase after detecting a sufficent time gap between sucessive vehicles. This allows for better distribution of green-time among phases and also affects cycle duration in response to dynamic traffic conditions.\"" | |||
| "To explain the actuated traffic ligjts, we refer to an excerpt from SUMO's documentation: \"SUMO supports gap-based actuated traffic control. This control scheme is common in Germany and works by prolonging traffic phases whenever a continuous stream of traffic is detected. It switches to the next phase after detecting a sufficent time gap between sucessive vehicles. This allows for better distribution of green-time among phases and also affects cycle duration in response to dynamic traffic conditions.\"" | |||
| ] | ||
| }, | ||
| { | ||
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "The difference between phases for static and actuated traffic lights is that actuated traffic light phases take in two additional parameters, `minDur` and `maxDur`, which describe the allowed range of time durations for each phase.\n", | ||
| "The difference between phases for static and actuated traffic lights is that actuated traffic light has two additional parameters in `phases`, namely `minDur` and `maxDur`, which describe the allowed range of time durations for each phase.\n", |
There was a problem hiding this comment.
Could you add that minDur is how long the phase will at least be held for, and that it will be held no longer than maxDur?
| "To control traffic lights via RL, no `tl_logic` element is necessary. This is because rllab is controlling all the parameters you were able to customize in the prior sections. Your `additional_net_params` should look something like this: " | ||
| "This is where we switch from the non-RL experiment script to the RL experiment. \n", | ||
| "\n", | ||
| "To control traffic lights via RL, no `tl_logic` element is necessary. This is because rllib is controlling all the parameters you were able to customize in the prior sections. The `additional_net_params` should look something like this: " |
There was a problem hiding this comment.
Hmm, don't think we should hardcode in RLlib. I think it's better to say that the RL agent is controlling the parameters.
| @@ -355,7 +383,7 @@ | |||
| "self.last_change = np.zeros((self.rows * self.cols, 1))\n", | |||
| "# keeps track of the direction of the intersection (the direction that is currently being allowed to flow. 0 indicates flow from top to bottom, and 1 indicates flow from left to right.)\n", | |||
| "self.direction = np.zeros((self.rows * self.cols, 1))\n", | |||
| "# value of 0 indicates that the intersection is in a red-yellow state. 1 indicates that the intersection is in a red-green state.\n", | |||
| "# value of 1 indicates that the intersection is in a red-yellow state. 0 indicates that the intersection is in a red-green state.\n", | |||
There was a problem hiding this comment.
nit: it's not clear what a red-yellow state is. You mean rYrY right? I think this is held over text so it's not your mistake but I think it's sort of unclear what it means.
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "In the case that the action space is discrete, we need 1-bit (that can be 0 or 1) for the action of each traffic light node. Hence, we need `self.num_traffic_lights` bits to represent the action spacs. To make a `self.num_traffic_lights`-bit number, we use the pyhton's `Discrete(range)`, and since we have `self.num_traffic_lights` bits, the `range` will be 2^`self.num_traffic_lights`.\n", |
| ] | ||
| }, | ||
| { | ||
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "### Observation Space" | ||
| "#### Observation Space" | ||
| ] | ||
| }, | ||
| { | ||
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": [ |
There was a problem hiding this comment.
Nit: it's not each vehicle, it's the N closest vehicles on each edge I think?
eugenevinitsky
approved these changes
Aug 19, 2019
|
I addressed all your concerns @eugenevinitsky! Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request information
Description
-- Revising the tutorial
-- Reordering of the sections
-- Adding new sections for reward and apply_rl_actions
READMEthe green_wave_env