Skip to content

[CI] Install dill in ci#116214

Closed
peterbell10 wants to merge 15 commits intogh/peterbell10/673/basefrom
gh/peterbell10/673/head
Closed

[CI] Install dill in ci#116214
peterbell10 wants to merge 15 commits intogh/peterbell10/673/basefrom
gh/peterbell10/673/head

Conversation

@peterbell10
Copy link
Copy Markdown
Collaborator

@peterbell10 peterbell10 commented Dec 20, 2023

[ghstack-poisoned]
@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Dec 20, 2023
@pytorch-bot
Copy link
Copy Markdown

pytorch-bot bot commented Dec 20, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/116214

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 6e2cf50 with merge base 5ec2d79 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
…ll in ci"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
@peterbell10 peterbell10 marked this pull request as ready for review December 22, 2023 18:47
@peterbell10 peterbell10 requested review from a team, ejguan and jeffdaily as code owners December 22, 2023 18:47
def test_multiprocessing_iterdatapipe(self):
self._test_multiprocessing_iterdatapipe(with_dill=False)

@unittest.expectedFailure
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This test fails in CI when the HAS_DILL code is active, so keeping the non-dill version active and xfailing the version with dill for now.

@lezcano lezcano removed their request for review December 24, 2023 11:17
@lezcano
Copy link
Copy Markdown
Collaborator

lezcano commented Jan 3, 2024

ping @malfet

Copy link
Copy Markdown
Contributor

@malfet malfet left a comment

Choose a reason for hiding this comment

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

Looks good to me, but it feels like this PR solves the problem when tests are running with dill, but does not test that PyTorch will not accidentally introduce unconditional dependency on dill, because its present in all CI configs.

Can we somehow keep one of the Linux CPU configs without dill(i.e. modify one of the test configs not uninstall dill before running the tests)

#Pinned versions:
#test that import:

dill
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.

Please specify the version that you want to use.

We have CI tests that depend on `dill` but none of the CI runners have it installed. This installs `dill` and fixes a bunch of broken tests and refactoring how dill is imported to avoid changing the default pickle behavior.

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
We have CI tests that depend on `dill` but none of the CI runners have it installed. This installs `dill` and fixes a bunch of broken tests and refactoring how dill is imported to avoid changing the default pickle behavior.

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
We have CI tests that depend on `dill` but none of the CI runners have it installed. This installs `dill` and fixes a bunch of broken tests and refactoring how dill is imported to avoid changing the default pickle behavior.

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
peterbell10 added a commit to peterbell10/pytorch that referenced this pull request Jan 24, 2024
ghstack-source-id: 3c853c4
Pull Request resolved: pytorch#116214
pytorchmergebot pushed a commit that referenced this pull request Jan 24, 2024
…6215)

This adds a reproducer for a failure that has since been fixed in main.

Pull Request resolved: #116215
Approved by: https://github.com/jansel
ghstack dependencies: #116230, #116214
@facebook-github-bot facebook-github-bot deleted the gh/peterbell10/673/head branch January 28, 2024 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants