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

aarch64: cd: remove conda dependency #1781

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

snadampal
Copy link
Contributor

@snadampal snadampal commented Apr 8, 2024

this solves the libomp performance issues.
fixes: #1774

Copy link

pytorch-bot bot commented Apr 9, 2024

No ciflow labels are configured for this repo.
For information on how to enable CIFlow bot see this wiki

@atalman
Copy link
Contributor

atalman commented Apr 9, 2024

@snadampal please open PR in pytorch/pytorch to test this change. Similar to this one:
https://github.com/pytorch/pytorch/pull/121979/files#diff-aa3d5c6114c91192e4f7d66f99f244ed95b08d00a140df29110056ae8d0a68a0L11

@snadampal snadampal force-pushed the aarch64_cd_update branch 3 times, most recently from aecee1a to 2fa4a66 Compare April 10, 2024 18:20
@snadampal snadampal marked this pull request as ready for review April 10, 2024 18:49
@snadampal
Copy link
Contributor Author

thanks, @atalman , I have raised this PR on pytorch/pytorch.

Copy link
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.

It looks like it tries to do two things at once: update docker and get rid of conda dependencies. With that in mind, do you mind splitting it into two (especially if you want to cherry-pick the libomp fix into 2.3 branch, as migration from manylinux2014 to manylinux2_28 is to big of a change to be accepted into release branch)

@snadampal
Copy link
Contributor Author

sure, I will split. but, as you know I migrated to manylinux2_28 so that it comes with all the packages that we were relying on conda earlier.
If we can't move to manylinux 2_28 for PyTorch 2.3 release, I will come up with a fix for libomp issue alone, but removing conda dependency fully on manylinux2014 may not be feasible.
so, in summary, I will do this:
release/2.3 : fix the libomp issue with current cd
main: first migrate to manylinux 2_28 and then remove conda depenency

@malfet
Copy link
Contributor

malfet commented Apr 12, 2024

sure, I will split. but, as you know I migrated to manylinux2_28 so that it comes with all the packages that we were relying on conda earlier.

Sorry, I might have missed the conversation. What packages do you need from 2_28, that you can not install in manylinux2014 from pypi instead of conda?

@snadampal
Copy link
Contributor Author

to start with, python3.10 itself, even the source builds were failing. probably if I spend more time, I will be able to figure out some workaround, but given manylinux 2014 EOL is June 30th, 2024, I have opted for manylinux 2_28.

@snadampal
Copy link
Contributor Author

snadampal commented Apr 18, 2024

libomp issue with the current CD is addressed in this PR: #1787

release/2.3 : fix the libomp issue with current cd

Once the above PR is merged, I will rebase and split the current PR into two steps for the below feature.

main: first migrate to manylinux 2_28 and then remove conda depenency

@snadampal snadampal changed the title aarch64: cd: upgrade to manylinux 2_28 and remove conda dependency aarch64: cd: remove conda dependency Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants