Skip to content

added outflow test MERGE BLOCKED BY KATHY #62

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

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

Conversation

eugenevinitsky
Copy link
Collaborator

  • Renamed all tests so that they are actually run by appending them with test
  • Added test for outflows
  • Do not merge this until @kjang96 approves because I've removed methods that she is currently using

@@ -290,51 +290,6 @@ def _split_edge(self, edge):
else:
return 0

def additional_command(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

did you mean to delete this? it's not mentioned in the overview or the commits

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, that's why this is blocked. Can we just leave this PR open and I'll move it later?

for veh_id in self.vehicles.get_ids():
self._reroute_if_final_edge(veh_id)

def _reroute_if_final_edge(self, veh_id):
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above

@@ -58,7 +58,7 @@ def test_collide(self):

def test_collide_inflows(self):
"""Tests collisions in the presence of inflows."""
# create the environment and scenario classes for a ring road
# create the environment and scenario classes for a 1x1 grid
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice catch

# test outflow rate
self.assertAlmostEqual(2*3600.0/15.0, vehicles.get_outflow_rate(100))

# test get num arrived
Copy link
Collaborator

Choose a reason for hiding this comment

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

are these coming later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, just filled them out for future

Copy link
Collaborator

@AboudyKreidieh AboudyKreidieh left a comment

Choose a reason for hiding this comment

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

LGTM, pending comments

kjang96 pushed a commit to kjang96/flow that referenced this pull request Aug 17, 2018
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