Skip to content

moved changes from flow-devel - #23

Merged
AboudyKreidieh merged 1 commit into
masterfrom
flow_devel_merge
Aug 10, 2018
Merged

moved changes from flow-devel#23
AboudyKreidieh merged 1 commit into
masterfrom
flow_devel_merge

Conversation

@AboudyKreidieh

Copy link
Copy Markdown
Collaborator

No description provided.

cathywu
cathywu previously requested changes Aug 10, 2018

@cathywucathywu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Did we want to get rid of docs/source/tutorial.rst as outdated?

Anyway, LGTM but probably @eugenevinitsky should take a pass as well.

rl_actions: list or numpy ndarray, optional
actions to be performed by rl vehicles in the network (if there
are any)
rl_actions: method, optional

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Cool 👍

# The maximum number of workers nodes to launch in addition to the head
# node. This takes precedence over min_workers.
max_workers: 2
max_workers: 0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why 0?

@eugenevinitskyeugenevinitskyAug 10, 2018

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This prevents it from auto-scaling; I think having extra nodes suddenly pop up and raise your costs would be undesired behavior.

"/tmp/foo2": "/Users/eugenevinitsky/.ssh/ray-autoscaler_1_us-west-1.pem",
#"/tmp/path": "<PATH TO LEARNING TRAFFIC>/.git/refs/heads/<BRANCH NAME>",
#"/tmp/ray_autoscaler_key": "<PATH TO RAY_AUTOSCALER_KEY>",
"/tmp/path": "/Users/eugenevinitsky/Desktop/Research/Bayen/Code/rllab-multiagent/learning-traffic/.git/refs/heads/ars_runner",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These paths probably shouldn't be committed, but this can be cleaned up in a separate PR. @eugenevinitsky

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A commit is coming. Noted.

- cd flow-devel && GIT_SSH_COMMAND="ssh -i ~/ray_autoscaler_key.pem" git fetch && GIT_SSH_COMMAND="ssh -i ~/ray_autoscaler_key.pem" git checkout `cat /tmp/foo`
- cd /tmp && cp ray_autoscaler_key ~/ray_autoscaler_key.pem
- cd flow-devel && GIT_SSH_COMMAND="ssh -i ~/ray_autoscaler_key.pem" git fetch && GIT_SSH_COMMAND="ssh -i ~/ray_autoscaler_key.pem" git checkout `cat /tmp/path`
- cd ray && git fetch eugene_upstream && git checkout add_ars && git pull

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same here @eugenevinitsky

@AboudyKreidiehAboudyKreidieh mentioned this pull request Aug 10, 2018
Comment threadREADME.md
- [Documentation](https://berkeleyflow.readthedocs.org/en/latest/)
- [Installation instructions](http://berkeleyflow.readthedocs.io/en/latest/flow_setup.html)
- [Tutorial](http://berkeleyflow.readthedocs.io/en/latest/tutorial.html)
- [Tutorials](https://github.com/berkeleyflow/flow/tree/master/tutorials)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What's happening here?

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

we're linking to the tutorials page on github instead of the old tutorial which is outdated and has been deleted

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

kk

@eugenevinitskyeugenevinitsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. Really nice work on the test environments

@AboudyKreidieh
AboudyKreidieh dismissed cathywu’s stale reviewAugust 10, 2018 20:24

eugene has commented on cathy's concerns

@AboudyKreidieh
AboudyKreidieh merged commit dc9c515 into masterAug 10, 2018
@AboudyKreidieh
AboudyKreidieh deleted the flow_devel_merge branch August 10, 2018 20:24
kjang96 pushed a commit to kjang96/flow that referenced this pull request Aug 17, 2018
Sign up for freeto 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

@AboudyKreidieh@cathywu@eugenevinitsky