Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Train/docs] Deduplicate Train examples & move from ray.air to ray.train #29667

Merged
merged 12 commits into from
Oct 27, 2022

Conversation

Yard1
Copy link
Member

@Yard1 Yard1 commented Oct 25, 2022

Signed-off-by: Antoni Baum [email protected]

Why are these changes needed?

This PR moves Train examples from ray.air.examples to ray.train.examples, making sure that duplicate examples are removed and the folder structure is preserved.

Related issue number

Closes #26536

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 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: Antoni Baum <[email protected]>
Signed-off-by: Antoni Baum <[email protected]>
Signed-off-by: Antoni Baum <[email protected]>
Signed-off-by: Antoni Baum <[email protected]>
Signed-off-by: Antoni Baum <[email protected]>
Copy link
Contributor

@amogkam amogkam left a comment

Choose a reason for hiding this comment

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

thanks, lgtm! just some minor comments

# args = ["--smoke-test"]
# )

# py_test(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this one already being tested in test_examples?

Copy link
Member Author

Choose a reason for hiding this comment

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

test_gpu_examples, will add a comment


# py_test(
# name = "tensorflow_autoencoder_example", # REGRESSION
# size = "medium",
Copy link
Contributor

Choose a reason for hiding this comment

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

this autoencoder example is still under the ray/air directory.

Let's keep this commented out pytest in the air BUILD file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, the .py file got moved here. The jupyter notebook is in ray/air, and that is not what this entry is referring to.

)

py_test(
name = "horovod_cifar_pbt_example",
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need an __init__.py in the ray/train/examples/horovod?

Copy link
Member Author

@Yard1 Yard1 Oct 26, 2022

Choose a reason for hiding this comment

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

I think we do. We may run into issues with imports (and we are importing functions from the examples) without it. It's present there.

@@ -1,7 +1,7 @@
import argparse

from ray.air.config import RunConfig, ScalingConfig
from ray.train.examples.torch_fashion_mnist_example import train_func
from ray.train.examples.pytorch.torch_fashion_mnist_example import train_func
from ray.train.torch import TorchTrainer
from ray.air.callbacks.mlflow import MLflowLoggerCallback
Copy link
Contributor

Choose a reason for hiding this comment

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

should this example be moved into pytorch?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess it is using pytorch, but on the other hand, it's supposed to show off MLFlow first and foremost and pytorch is only an implementation detail here. I'd just leave it where it is for better visibility.

Signed-off-by: Antoni Baum <[email protected]>
@amogkam
Copy link
Contributor

amogkam commented Oct 27, 2022

Failing tests are also failing on master. Going to merge.

@amogkam amogkam merged commit b44f5cd into ray-project:master Oct 27, 2022
WeichenXu123 pushed a commit to WeichenXu123/ray that referenced this pull request Dec 19, 2022
….train` (ray-project#29667)

This PR moves Train examples from ray.air.examples to ray.train.examples, making sure that duplicate examples are removed and the folder structure is preserved.

Signed-off-by: Antoni Baum <[email protected]>
Signed-off-by: Weichen Xu <[email protected]>
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.

[AIR/Docs] Deduplicate AIR-Train examples
3 participants