Migrate torch::deploy test infra to pytorch/test-infra #274
Conversation
facebook-github-bot
added
the
CLA Signed
| pip install -e . | ||
| python multipy/runtime/example/generate_examples.py | ||
|
|
||
| # Test |
There was a problem hiding this comment.
Is it possible to split these up into different jobs? You should be able to create a dependency from the build and the other jobs. Just make sure tarball creation + distribution is dependent on the tests passing.
There was a problem hiding this comment.
Yes, that would be possible! However, each step using the linux-job from pytest/test-infra begins with a plain conda-builder image. Therefore, each would need to install dependencies and build MultiPy separately which is quite expensive I'm afraid.
I added what I believe to be the next best thing which is log groups (sections that can be collapsed):
There was a problem hiding this comment.
ohh that's great. We just want to make sure we have a good way to split things up :)
| # Create artefact to be uploaded | ||
| mkdir dist | ||
| mv ${{ inputs.release-artefact }}.tar.gz \ | ||
| ${{ inputs.release-artefact }}.whl |
There was a problem hiding this comment.
Just add a comment for why you are doing the renaming as its not intuitive and relies on an implementation detail. I'll bug the code owner (@seemethere) to see if I can add tar support to simplify this.
There was a problem hiding this comment.
Yes, great idea! I added a comment with a todo to remove the renaming once pytorch/test-infra#1262 is merged.
|
@hatala91 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@hatala91 has updated the pull request. You must reimport the pull request before landing. |
| runs-on: ubuntu-latest | ||
| if: ${{ inputs.release }} | ||
| steps: | ||
| - name: Download artefact |
multipy/runtime/loader.cpp
Outdated
| @@ -67,6 +67,14 @@ | |||
| #include <multipy/runtime/loader.h> | |||
| #include <multipy/runtime/mem_file.h> | |||
|
|
|||
| /* Linux traditionally doesn't have the trailing 64 that BSD has on these. */ | |||
There was a problem hiding this comment.
Is this for CentoOS 7? Can you a bit more clear on why its needed.
There was a problem hiding this comment.
Great catch, I copied the whole block and forgot to adapt the comment.
| echo "::endgroup::" | ||
|
|
||
| echo "::group::Move artifact for upload" | ||
| mv multipy_runtime_python${python_version}.tar.gz ${RUNNER_ARTIFACT_DIR}/ |
There was a problem hiding this comment.
@seemethere had some comments on your test-infra PR as a way to improve this. Happy to approve after you fix that.
There was a problem hiding this comment.
Adopted (even pushed that commit shortly before you commented) and can confirm that moving the artifact directly to that folder works excellent:
|
@hatala91 has updated the pull request. You must reimport the pull request before landing. |
|
@hatala91 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
There was a problem hiding this comment.
Nice! This is amazing.
As some logistical asides before landing. Please add a summary to describe your changes. (I think the issue should give decent motivation, but its generally good practice to add a few sentences or more describing what you did.)
Also add some jobs to the summary to show that release and build-test work as intended.
|
@hatala91 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: With this PR, I propose to migrate [runtime_nightly.yaml](https://github.com/pytorch/multipy/blob/main/.github/workflows/runtime_nightly.yaml) and [runtime_tests.yaml](https://github.com/pytorch/multipy/blob/main/.github/workflows/runtime_tests.yaml) to use [pytorch/test_infra](https://github.com/pytorch/test-infra/wiki/). For this, I introduce a new reusable workflow in [build_test_release.yaml](https://github.com/pytorch/multipy/pull/274/files#diff-e9376faa19019165b5c9299c6e79ef11820184ea067877dd574bf357b7a7e01b) which is called from both the runtime tests and nightly build pipeline. The workflow then uses `pytorch/test_infra's` [linux_job.yml](https://github.com/pytorch/test-infra/blob/main/.github/workflows/) to build, test, run examples and benchmark. Example runs: - py37: https://github.com/pytorch/multipy/actions/runs/3708138595/jobs/6285368845 - py38: https://github.com/pytorch/multipy/actions/runs/3708138595/jobs/6285368958 - py39: https://github.com/pytorch/multipy/actions/runs/3708138595/jobs/6285369062 - py310: https://github.com/pytorch/multipy/actions/runs/3708138595/jobs/6285368633 Issue: #254 Pull Request resolved: #274 Reviewed By: seemethere, PaliC Differential Revision: D42069719 Pulled By: hatala91 fbshipit-source-id: b24162f9436809a5688e835d50f18212214b5c48
|
This pull request was exported from Phabricator. Differential Revision: D42069719 |
With this PR, I propose to migrate runtime_nightly.yaml and runtime_tests.yaml to use pytorch/test_infra.
For this, I introduce a new reusable workflow in build_test_release.yaml which is called from both the runtime tests and nightly build pipeline. The workflow then uses
pytorch/test_infra'slinux_job.yml to build, test, run examples and benchmark.Example runs:
Issue: #254