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

scenarios -> networks [do not merge] #698

Merged
merged 43 commits into from Sep 12, 2019
Merged

Conversation

AboudyKreidieh
Copy link
Member

Pull request information

  • Status: ? (ready to merge / in development)
  • Kind of changes: ? (bug fix / new feature / documentation...)
  • Related PR or issue: ? (optional)

Description

? (general description)

@AboudyKreidieh AboudyKreidieh changed the title Aboudy/scenario networks scenarios -> networks Aug 20, 2019
Copy link
Member

@ashkan-software ashkan-software left a comment

Choose a reason for hiding this comment

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

Everything looks great!

README.md Show resolved Hide resolved
flow/core/kernel/kernel.py Show resolved Hide resolved
flow/core/kernel/network/__init__.py Outdated Show resolved Hide resolved
flow/core/kernel/scenario/__init__.py Show resolved Hide resolved
flow/networks/merge.py Outdated Show resolved Hide resolved
@ashkan-software
Copy link
Member

@AboudyKreidieh I did a search for scenario in the whole flow folder. Looks like not all scenarios are replaced with network. Could you look into this?

Screen Shot 2019-08-21 at 00 23 56

@ashkan-software
Copy link
Member

I can also take care of these changes (scenario to network) if you like @AboudyKreidieh

@AboudyKreidieh AboudyKreidieh changed the title scenarios -> networks scenarios -> networks [do not merge] Aug 24, 2019
@AboudyKreidieh
Copy link
Member Author

AboudyKreidieh commented Aug 24, 2019

@AboudyKreidieh I did a search for scenario in the whole flow folder. Looks like not all scenarios are replaced with network. Could you look into this?

Screen Shot 2019-08-21 at 00 23 56

@ashkan-software I removed all instances of scenario that I thought were relevant, except for the AImsun stuff which I tried to avoid breaking. If you see any specific uses of scenario that you think are wrong I'd be happy to discuss with you.

@coveralls
Copy link

coveralls commented Aug 27, 2019

Pull Request Test Coverage Report for Build 4417

  • 1098 of 1207 (90.97%) changed or added relevant lines in 91 files are covered.
  • 12 unchanged lines in 4 files lost coverage.
  • Overall coverage decreased (-0.9%) to 90.936%

Changes Missing Coverage Covered Lines Changed/Added Lines %
flow/envs/base.py 14 15 93.33%
flow/envs/bottleneck.py 19 20 95.0%
flow/envs/ring/wave_attenuation.py 5 6 83.33%
flow/networks/traffic_light_grid.py 197 198 99.49%
flow/visualize/visualizer_rllib.py 0 1 0.0%
flow/core/kernel/vehicle/traci.py 18 20 90.0%
flow/core/rewards.py 0 2 0.0%
flow/networks/base.py 102 104 98.08%
flow/scenarios/base_scenario.py 0 2 0.0%
flow/scenarios/grid.py 0 3 0.0%
Files with Coverage Reduction New Missed Lines %
flow/scenarios/figure_eight.py 3 0.0%
flow/scenarios/grid.py 3 0.0%
flow/scenarios/loop.py 3 0.0%
flow/scenarios/multi_loop.py 3 0.0%
Totals Coverage Status
Change from base Build 4415: -0.9%
Covered Lines: 9130
Relevant Lines: 10040

💛 - Coveralls

@AboudyKreidieh
Copy link
Member Author

since @ashkan-software has already reviewed this and it is passing the tests I'm gonna go ahead and merge it

@AboudyKreidieh AboudyKreidieh merged commit bd49a78 into master Sep 12, 2019
@AboudyKreidieh AboudyKreidieh deleted the aboudy/scenario_networks branch September 12, 2019 08:36
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.

None yet

4 participants