{"action":"created","issue":{"url":"https://api.github.com/repos/Azure/BatchAI/issues/15","repository_url":"https://api.github.com/repos/Azure/BatchAI","labels_url":"https://api.github.com/repos/Azure/BatchAI/issues/15/labels{/name}","comments_url":"https://api.github.com/repos/Azure/BatchAI/issues/15/comments","events_url":"https://api.github.com/repos/Azure/BatchAI/issues/15/events","html_url":"https://github.com/Azure/BatchAI/issues/15","id":283800008,"node_id":"MDU6SXNzdWUyODM4MDAwMDg=","number":15,"title":"Provide TF_CONFIG environment variable for distributed TensorFlow","user":{"login":"damienpontifex","id":1321276,"node_id":"MDQ6VXNlcjEzMjEyNzY=","avatar_url":"https://avatars1.githubusercontent.com/u/1321276?v=4","gravatar_id":"","url":"https://api.github.com/users/damienpontifex","html_url":"https://github.com/damienpontifex","followers_url":"https://api.github.com/users/damienpontifex/followers","following_url":"https://api.github.com/users/damienpontifex/following{/other_user}","gists_url":"https://api.github.com/users/damienpontifex/gists{/gist_id}","starred_url":"https://api.github.com/users/damienpontifex/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/damienpontifex/subscriptions","organizations_url":"https://api.github.com/users/damienpontifex/orgs","repos_url":"https://api.github.com/users/damienpontifex/repos","events_url":"https://api.github.com/users/damienpontifex/events{/privacy}","received_events_url":"https://api.github.com/users/damienpontifex/received_events","type":"User","site_admin":false},"labels":[{"id":691899852,"node_id":"MDU6TGFiZWw2OTE4OTk4NTI=","url":"https://api.github.com/repos/Azure/BatchAI/labels/enhancement","name":"enhancement","color":"84b6eb","default":true}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":20,"created_at":"2017-12-21T07:54:35Z","updated_at":"2018-09-21T12:47:51Z","closed_at":null,"author_association":"NONE","body":"The TensorFlow `ClusterConfig` can parse worker and parameter server settings from a `TF_CONFIG` environment variable (see https://github.com/tensorflow/tensorflow/blob/master/tensorflow/contrib/learn/python/learn/estimators/run_config.py#L64-L156)\r\n\r\nI was trying to pass it via an environment variable in the job configuration file like so:\r\n```json\r\n\"{ 'cluster': { 'ps': $AZ_BATCHAI_PS_HOSTS, 'worker': $AZ_BATCHAI_WORKER_HOSTS }, 'task': { 'index': $AZ_BATCHAI_TASK_INDEX, 'type': '' } }\"\r\n```\r\n\r\nWhich is kind of fine, but falls down for a few cases:\r\n1. When there are no parameter servers (i.e. single node) the ps hosts should be an empty array, but in this case it's just an empty string.\r\n2. The variables for hosts and workers are comma separated and the TF code parses it as JSON, so would ideally be an array type inside this string.\r\n2. The 'task.type' property can be 'master', 'worker' or 'ps' but that doesn't seem to have an appropriate environment variable and I had to pass the option via command line args\r\n\r\nMore generally though, providing this configuration via a `TF_CONFIG` environment variable would significantly lower the bar to get distributed training working in TensorFlow and Azure Batch. It would also simplify command line arg parameters and mean just the appropriate data directories would need to be passed and mean the same arguments could be used across master, worker and ps potentially simplify the tensorflowSettings property further."},"comment":{"url":"https://api.github.com/repos/Azure/BatchAI/issues/comments/423519281","html_url":"https://github.com/Azure/BatchAI/issues/15#issuecomment-423519281","issue_url":"https://api.github.com/repos/Azure/BatchAI/issues/15","id":423519281,"node_id":"MDEyOklzc3VlQ29tbWVudDQyMzUxOTI4MQ==","user":{"login":"damienpontifex","id":1321276,"node_id":"MDQ6VXNlcjEzMjEyNzY=","avatar_url":"https://avatars1.githubusercontent.com/u/1321276?v=4","gravatar_id":"","url":"https://api.github.com/users/damienpontifex","html_url":"https://github.com/damienpontifex","followers_url":"https://api.github.com/users/damienpontifex/followers","following_url":"https://api.github.com/users/damienpontifex/following{/other_user}","gists_url":"https://api.github.com/users/damienpontifex/gists{/gist_id}","starred_url":"https://api.github.com/users/damienpontifex/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/damienpontifex/subscriptions","organizations_url":"https://api.github.com/users/damienpontifex/orgs","repos_url":"https://api.github.com/users/damienpontifex/repos","events_url":"https://api.github.com/users/damienpontifex/events{/privacy}","received_events_url":"https://api.github.com/users/damienpontifex/received_events","type":"User","site_admin":false},"created_at":"2018-09-21T12:47:51Z","updated_at":"2018-09-21T12:47:51Z","author_association":"NONE","body":"I was looking at what is currently being set and what changes need to make the `RunConfig` parse it correctly. Investigations outlined below and will look into updating the TF_CONFIG variable on each machine through code to ensure this change is successful. @lliimsft could this below help in making the appropriate changes? \r\n\r\nTo verify what JSON structure worked I setup:\r\n\r\n```python\r\nos.environ['TF_CONFIG'] = TF_CONFIG_JSON_STRING\r\n\r\nconfig = tf.estimator.RunConfig()\r\nprint('master => {}'.format(config.master))\r\nprint('task_id => {}'.format(config.task_id))\r\nprint('num_ps_replicas => {}'.format(config.num_ps_replicas))\r\nprint('num_worker_replicas => {}'.format(config.num_worker_replicas))\r\nprint('cluster_spec => {}'.format(config.cluster_spec))\r\nprint('task_type => {}'.format(config.task_type))\r\nprint('is_chief => {}'.format(config.is_chief))\r\n```\r\n\r\nRun with a 3 node job configured with 1 parameter server and 3 worker count\r\n\r\n### Current\r\nCurrently in Batch AI we get the TF_CONFIG environment variable being:\r\nIn ps-0\r\n```json \r\n{\"cluster\":{\"ps\":[\"10.0.0.4:2222\"],\"worker\":[\"10.0.0.4:2223\",\"10.0.0.5:2222\",\"10.0.0.6:2222\"]},\"task\":{\"type\":\"ps\",\"index\":0},\"environment\":\"cloud\"}\r\n```\r\nwk-0\r\n```json\r\n{\"cluster\":{\"ps\":[\"10.0.0.4:2222\"],\"worker\":[\"10.0.0.4:2223\",\"10.0.0.5:2222\",\"10.0.0.6:2222\"]},\"task\":{\"type\":\"worker\",\"index\":0},\"environment\":\"cloud\"}\r\n```\r\nwk-1\r\n```json\r\n{\"cluster\":{\"ps\":[\"10.0.0.4:2222\"],\"worker\":[\"10.0.0.4:2223\",\"10.0.0.5:2222\",\"10.0.0.6:2222\"]},\"task\":{\"type\":\"worker\",\"index\":1},\"environment\":\"cloud\"}\r\n```\r\nwk-2\r\n```json\r\n{\"cluster\":{\"ps\":[\"10.0.0.4:2222\"],\"worker\":[\"10.0.0.4:2223\",\"10.0.0.5:2222\",\"10.0.0.6:2222\"]},\"task\":{\"type\":\"worker\",\"index\":2},\"environment\":\"cloud\"}\r\n```\r\n\r\nWith these, the python code above gave the error:\r\n```\r\nValueError: If \"cluster\" is set in TF_CONFIG, it must have one \"chief\" node.\r\n```\r\n\r\n### Working\r\nTo get this working, we apparently need the master worker defined under chief in the cluster. As such, the 'cluster' part of the JSON object would become:\r\n```json\r\n\"cluster\":{\"chief\": [\"10.0.0.4:2223\"],\"ps\":[\"10.0.0.4:2222\"],\"worker\":[\"10.0.0.5:2222\",\"10.0.0.6:2222\"]}\r\n```\r\nThen the task component would be changed for whichever node is initiated from `masterCommandLineArgs` and have task of:\r\n```json\r\n\"task\":{\"type\":\"chief\",\"index\":0}\r\n```\r\n\r\nThe other worker nodes would have the same as before with index now being 0 or 1 e.g.\r\n```json\r\n\"task\":{\"type\":\"worker\",\"index\":1}\r\n```\r\n\r\n### Testing\r\nThis sample code parses into the `RunConfig` correctly, but I haven't tested it on a cluster and an estimator yet to see if it hooks it all up fine:\r\n```python\r\nimport os\r\nimport json\r\nimport tensorflow as tf\r\n\r\ndef log_config_for(runconfig_string):\r\n  os.environ['TF_CONFIG'] = runconfig_string\r\n\r\n  config = tf.estimator.RunConfig()\r\n  print('master => {}'.format(config.master))\r\n  print('task_id => {}'.format(config.task_id))\r\n  print('num_ps_replicas => {}'.format(config.num_ps_replicas))\r\n  print('num_worker_replicas => {}'.format(config.num_worker_replicas))\r\n  print('cluster_spec => {}'.format(config.cluster_spec))\r\n  print('task_type => {}'.format(config.task_type))\r\n  print('is_chief => {}'.format(config.is_chief))\r\n  print()\r\n\r\ndef main():\r\n\r\n  machine_definitions = [ \r\n    # Machine expected from settings with parameterServerCommandLineArgs\r\n    '{\"cluster\":{\"chief\": [\"10.0.0.4:2223\"],\"ps\":[\"10.0.0.4:2222\"],\"worker\":[\"10.0.0.5:2222\",\"10.0.0.6:2222\"]},\"task\":{\"type\":\"ps\",\"index\":0},\"environment\":\"cloud\"}',\r\n    # Machine expected from settings with masterCommandLineArgs\r\n    '{\"cluster\":{\"chief\": [\"10.0.0.4:2223\"],\"ps\":[\"10.0.0.4:2222\"],\"worker\":[\"10.0.0.5:2222\",\"10.0.0.6:2222\"]},\"task\":{\"type\":\"chief\",\"index\":0},\"environment\":\"cloud\"}',\r\n    # Machine expected from settings with workerCommandLineArgs\r\n    '{\"cluster\":{\"chief\": [\"10.0.0.4:2223\"],\"ps\":[\"10.0.0.4:2222\"],\"worker\":[\"10.0.0.5:2222\",\"10.0.0.6:2222\"]},\"task\":{\"type\":\"worker\",\"index\":0},\"environment\":\"cloud\"}',\r\n    # Machine expected from settings with workerCommandLineArgs\r\n    '{\"cluster\":{\"chief\": [\"10.0.0.4:2223\"],\"ps\":[\"10.0.0.4:2222\"],\"worker\":[\"10.0.0.5:2222\",\"10.0.0.6:2222\"]},\"task\":{\"type\":\"worker\",\"index\":1},\"environment\":\"cloud\"}'\r\n  ]\r\n\r\n  for definition in machine_definitions:\r\n    log_config_for(definition)\r\n\r\nif __name__ == '__main__':\r\n  main()\r\n```"}}
{"actor":{"display_login":"damienpontifex"}}