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

New inflow #911

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

New inflow #911

wants to merge 10 commits into from

Conversation

AboudyKreidieh
Copy link
Member

@AboudyKreidieh AboudyKreidieh commented Apr 20, 2020

Pull request information

  • Status: ready to merge
  • Kind of changes: new features
  • Related PR or issue: None

Description

  • added an ignore_noise option to the controllers to deactivate noise near the start of networks
  • moved the "vehsPerHour" version of the inflows from sumo's interface to the VehicleParams class. This gives us more control on how vehicles enter the network.
  • modified the edgestarts to the highway network. This is needed when calling the ignore_noise feature.

@AboudyKreidieh AboudyKreidieh changed the title New inflow (do not merge) New inflow Apr 22, 2020
@coveralls
Copy link

coveralls commented Apr 22, 2020

Pull Request Test Coverage Report for Build 5763

  • 69 of 72 (95.83%) changed or added relevant lines in 7 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.02%) to 89.473%

Changes Missing Coverage Covered Lines Changed/Added Lines %
flow/controllers/base_controller.py 7 10 70.0%
Files with Coverage Reduction New Missed Lines %
flow/core/kernel/network/traci.py 1 91.06%
Totals Coverage Status
Change from base Build 5755: 0.02%
Covered Lines: 9383
Relevant Lines: 10487

💛 - Coveralls

Copy link
Collaborator

@kanaadp kanaadp left a comment

Choose a reason for hiding this comment

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

Have you checked the performance of manually adding each vehicle vs specifiying an inflow? Seems like it could cause significantly more traci calls?

Otherwise, seems fine. I think this will be really useful.

accel += np.random.normal(0, self.accel_noise)
if self.ignore_noise is None:
# Add noise to the vehicle for all positions in this case.
accel += np.random.normal(0, self.accel_noise)
Copy link
Collaborator

Choose a reason for hiding this comment

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

realized that this should probably be seeded with SUMO seed as well.

edge=self._inflows[name]["edge"],
pos=0,
lane=self._inflows[name]["departLane"],
speed=depart_speed
Copy link
Collaborator

Choose a reason for hiding this comment

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

how does this work if adding a vehicle at the beginning of the lane with the given depart_speed is unsafe? Does it have the same behavior as SUMO inflows (either wait, adapt, or throw away the existing vehicle)?

@@ -13,6 +13,8 @@
from bisect import bisect_left
import itertools
from copy import deepcopy
import random
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, we should sync the seeds to get reproducibility

name = names[i + 1]

# Choose the departure speed.
depart_speed = self._inflows[name]["departSpeed"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this also be adaptive?

else 0)
)

for veh_num in range(num_vehicles):
Copy link
Collaborator

Choose a reason for hiding this comment

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

at each time step, we should shuffle the vehicles to be added so they're not added in batches (i.e. 5 humans, then 2 avs)

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.

3 participants