Skip to content

Conversation

AboudyKreidieh
Copy link
Member

No description provided.

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.

LGTM - see some small comments.

""" The below methods need to be updated by child classes. """

def _apply_rl_actions(self, rl_actions):
"""Update by child classes."""
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 read "update using child classes" or "implemented by child classes"?

"""
Partial observed state space. Velocities, distance to intersections,
edge number (for nearby vehicles) traffic light state
Partial observed state space.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Super nitpicky, but should this be "Partially observed"?

Copy link
Member

Choose a reason for hiding this comment

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

Not too nitpicky, this is a good catch.

kjang96 pushed a commit to kjang96/flow-1 that referenced this pull request Sep 1, 2018
Copy link
Member

@eugenevinitsky eugenevinitsky left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @nskh for the review.

@eugenevinitsky eugenevinitsky merged commit 0ed609b into master Sep 3, 2018
@eugenevinitsky eugenevinitsky deleted the doctring_envs branch September 3, 2018 19:45
crystalzyan pushed a commit to crystalzyan/flow that referenced this pull request Dec 1, 2018
* cleaned up docstring in envs

* added Nish fixes
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