Skip to content

Multiagent pr 2#270

Merged
eugenevinitsky merged 30 commits intomasterfrom
multiagent_pr_2
Nov 26, 2018
Merged

Multiagent pr 2#270
eugenevinitsky merged 30 commits intomasterfrom
multiagent_pr_2

Conversation

@eugenevinitsky
Copy link
Member

@eugenevinitsky eugenevinitsky commented Nov 20, 2018

Separate PR for the multi-agent upgrades. @AboudyKreidieh I felt for you.

  • two multiagent examples: an adversarial example and an example of shared moels
  • compliance of visual_rllib with multiagent
  • complain of base_env with multiagent

@coveralls
Copy link

coveralls commented Nov 20, 2018

Pull Request Test Coverage Report for Build 1616

  • 506 of 631 (80.19%) changed or added relevant lines in 21 files are covered.
  • 7 unchanged lines in 5 files lost coverage.
  • Overall coverage decreased (-0.03%) to 79.436%

Changes Missing Coverage Covered Lines Changed/Added Lines %
flow/scenarios/multi_loop.py 90 91 98.9%
flow/visualize/visualizer_rllib.py 52 53 98.11%
tests/fast_tests/test_examples.py 11 12 91.67%
flow/envs/loop/loop_accel.py 22 24 91.67%
tests/fast_tests/test_visualizers.py 11 14 78.57%
examples/rllib/multiagent_exps/multiagent_figure_eight.py 57 61 93.44%
examples/rllib/multiagent_exps/multiagent_stabilizing_the_ring.py 53 57 92.98%
flow/core/vehicles.py 18 22 81.82%
flow/envs/base_env.py 22 45 48.89%
flow/envs/loop/wave_attenuation.py 19 55 34.55%
Files with Coverage Reduction New Missed Lines %
flow/envs/loop/wave_attenuation.py 1 46.09%
flow/visualize/visualizer_rllib.py 1 86.67%
flow/envs/base_env.py 1 74.6%
flow/envs/bay_bridge/base.py 2 62.39%
flow/core/vehicles.py 2 89.53%
Totals Coverage Status
Change from base Build 1592: -0.03%
Covered Lines: 6710
Relevant Lines: 8447

💛 - Coveralls

@eugenevinitsky
Copy link
Member Author

Don't merge; I just wanted to check that the tests will pass. Will be ready for merge tonight after additional tests are added.

Copy link
Member

@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.

please name multiagent_stabilizing_the_ring.py -> lord_of_the_rings.py

if isinstance(veh_id, (list, np.ndarray)):
return [self.get_lane_followers_speed(vehID, error)
for vehID in veh_id]
return self.__vehicles.get(veh_id, {}).get("followers_speed", error)
Copy link
Member

Choose a reason for hiding this comment

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

same as above

Copy link
Member

@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.

from a high level, when is base_env.py used and when is multiagent_env.py used?

Copy link
Member

@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.

nevermind

Copy link
Member

@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! please fix the comments first, but then feel free to merge

@eugenevinitsky
Copy link
Member Author

image

HAHAHA

@eugenevinitsky
Copy link
Member Author

I'll also add some descriptions of this to the documentation first

@eugenevinitsky
Copy link
Member Author

@AboudyKreidieh we should have this discussion before I merge:
MultiEnv is for when you have multiple agents, everything else just subclasses Env. I am adding examples of this to the docs. There doesn't appear to be a cleaner solution at the moment.

@eugenevinitsky eugenevinitsky merged commit fbafbad into master Nov 26, 2018
@eugenevinitsky eugenevinitsky deleted the multiagent_pr_2 branch November 26, 2018 16:26
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