Skip to content

[RLlib] Change default framework from tf to torch#33604

Merged
gjoliver merged 11 commits into
ray-project:masterfrom
kouroshHakha:change-default-from-tf-to-torch
Mar 24, 2023
Merged

[RLlib] Change default framework from tf to torch#33604
gjoliver merged 11 commits into
ray-project:masterfrom
kouroshHakha:change-default-from-tf-to-torch

Conversation

@kouroshHakha
Copy link
Copy Markdown
Contributor

Why are these changes needed?

This PR changes the default framework_str from tf to either torch or tf2. First step towards hopefully deprecating tf1 stack.

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@kourosh-JRFKXJ33VL.local.meter>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@kourosh-JRFKXJ33VL.local.meter>

# `self.framework()`
self.framework_str = "tf"
self.framework_str = "torch"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:fingers-crossed: :)

Copy link
Copy Markdown
Contributor

@sven1977 sven1977 left a comment

Choose a reason for hiding this comment

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

Looks great! Some example scripts used to run only on tf and will now only run on torch, but I guess that's ok. E.g. custom_metrics_and_callbacks.

Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
# __export-models-as-onnx-begin__
# Using the same Policy object, we can also export our NN Model in the ONNX format:
ppo_policy.export_model("/tmp/my_nn_model", onnx=True)
ppo_policy.export_model("/tmp/my_nn_model", onnx=False)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

update comment?

Comment thread rllib/BUILD
name = "connectors/tests/test_agent",
tags = ["team:rllib", "connector"],
size = "small",
size = "medium",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is from some other pr right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The other PR will get merged and this difference will go away. I wanted to make sure the tests on CI doesn't get red b/c of time outs.

env_creator=lambda _: MultiAgentCartPole({"num_agents": 2}),
default_policy_class=ModelBasedPolicy,
config=DQNConfig()
.framework("tf")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

curious, multi-agent env doesn't work with torch?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It does. This test is overfitted to tf.

@gjoliver
Copy link
Copy Markdown
Member

ok ok

@gjoliver gjoliver merged commit 8d2dc9a into ray-project:master Mar 24, 2023
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
* changed default in algo config
* implicitly added tf framework to the test scripts

Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: elliottower <elliot@elliottower.com>
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request May 4, 2023
* changed default in algo config
* implicitly added tf framework to the test scripts

Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Jack He <jackhe2345@gmail.com>
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.

5 participants