Rework example and e2e test script #126
Conversation
|
Hi @TimZaman. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/ok-to-test |
|
@TimZaman Great work. Thanks for your contributions Few comments
/cc @andreyvelich |
|
Ok thanks guys, I'll take it that this PR is appreciated (it does update a ton of stuff in examples so wasn't sure). I'll move forward to address comments and remove the WIP state. |
- Unify many copies of mnist training scripts into a single one. - Backends: Gloo, NCCL, MPI. - Bases of the official pytorch/examples/mnist example. - Uses TensorboardX for summary writing. - Uses a backend argument to select backend (if applicable). - Update all examples and dockerfiles to Pytorch 1.0. - Remove examples using deprecated TCP backend. - Remove old v1alpha1 examples.
|
All comments addressed. As I'm unfamiliar with this project's CI process, I'll see how things go wrt the |
|
@TimZaman CI tests fail because mnist tests have failed. https://github.com/kubeflow/pytorch-operator/blob/master/scripts/v1alpha2/run-defaults.sh#L49 Each test have a timeout of 10 minutes. I think, tests fail because they won't get completed in 10 minutes and they time out. Since we are not concerned about the accuracy of the example, we can keep the default epoch of the example to be 1(https://github.com/kubeflow/pytorch-operator/pull/126/files#diff-f5284f418f68386cd925f0081ba5dcc9R83). WDYT? |
|
SGTM, how do you know its timeouts?
…On Thu, Jan 17, 2019 at 12:31 AM Johnu George ***@***.***> wrote:
@TimZaman <https://github.com/TimZaman> CI tests fail because mnist tests
have failed.
https://github.com/kubeflow/pytorch-operator/blob/master/scripts/v1alpha2/run-defaults.sh#L49
https://github.com/kubeflow/pytorch-operator/blob/master/scripts/v1alpha2/run-cleanpodpolicy-all.sh#L50
Each test have a timeout of 10 minutes. I think, tests fail because they
won't get completed in 10 minutes and they time out. Since we are not
concerned about the accuracy of the example, we can keep the default epoch
of the example to be 1(
https://github.com/kubeflow/pytorch-operator/pull/126/files#diff-f5284f418f68386cd925f0081ba5dcc9R83).
WDYT?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#126 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHXSRE0yo_PuntXiy7OWW7wfbwLqGz1zks5vEDTYgaJpZM4Z62nH>
.
|
Should help reduce the change of CI timeout
|
Timeout is a parameter in the test. |
|
/retest |
|
Tests passed. Thanks @TimZaman /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: johnugeorge The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Submitted as WIP, everything working, just wrapping up my tests for the mpi backend. Let me know if this is OKAY. There is quite some redundancy/small bugs/outdated/unmaintained in the examples dir, so I decided to take a stab to clean it up! :-)
(Also I don't think maybe the MPI backend should be promoted, the kubeflow/MPI-operator is superior for this purpose: tailored for MPI and topology awareness).
This change is