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

Change _apply_rl_actions in green wave and added tests #42

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kjang96
Copy link
Collaborator

@kjang96 kjang96 commented Aug 16, 2018

Removed switch_time from green_wave_env's env params, and replaced with min_yellow_time and min_green_time. Also added tests in test_traffic_lights.py

self.last_change is still 3 columns long, but now records last change for switches to green, not only switches to yellow.

…th min_yellow_time and min_green_time. Also added tests in test_traffic_lights.py
Copy link
Collaborator

@nskh nskh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments. Also, looks like you missed a merge conflict

@@ -71,6 +72,18 @@ def run_task(*_):
num_cars_right = 1
num_cars_top = 1
num_cars_bot = 1
=======
v_enter = 20
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've got merge conflicts in this file.

if action:
else: # currently green
self.last_change[i, 0] += self.sim_step
if self.last_change[i, 0] >= self.min_green_time and rl_mask[i]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does self.last_change get incremented inside the else when it didn't before? (not too familiar with the green wave stuff though so I might just be missing something obvious)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.last_change used to only keep track of last change since a yellow phase began. Now it also keeps track of time since a green phase began. So now, last_change[i, 0] resets when the green phase begins (it didn't use to) and increments during the green phase as well.

@@ -194,6 +195,45 @@ def test_k_closest(self):
self.assertTrue(self.env.vehicles.get_edge(veh_id) in c0_edges)


def test_min_switch(self):
# FOR THE PURPOSES OF THIS TEST, never set min switch to be < 2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this test enforce the constraint of min_switch >= 2?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setup script it uses has min_switch >=2, is that what you mean?

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

Successfully merging this pull request may close these issues.

2 participants