Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Could you release the sdist on pypi besides the wheels #19776

Open
KaixuYang opened this issue Jan 22, 2021 · 21 comments
Open

Could you release the sdist on pypi besides the wheels #19776

KaixuYang opened this issue Jan 22, 2021 · 21 comments

Comments

@KaixuYang
Copy link

KaixuYang commented Jan 22, 2021

Hello,

For the python version, would it be possible to release the sdist files on pypi besides the wheel files? Thanks!

@github-actions
Copy link

Welcome to Apache MXNet (incubating)! We are on a mission to democratize AI, and we are glad that you are contributing to it by opening this issue.
Please make sure to include all the relevant context, and one of the @apache/mxnet-committers will be here shortly.
If you are interested in contributing to our project, let us know! Also, be sure to check out our guide on contributing to MXNet and our development guides wiki.

@szha
Copy link
Member

szha commented Jan 22, 2021

I think that's a good idea. With the revamped cmake build, the build system could potentially auto-discover available libraries for integration when building. The relevant work would be to add the logic for triggering cmake build into the setup.py.

@leezu
Copy link
Contributor

leezu commented Jan 22, 2021

@KaixuYang would you like to help improve the setup.py to make this possible. I'm happy to advise on any questions

@KaixuYang
Copy link
Author

@leezu I have no background in this but I would like to give it a try if that’s something I could learn in a short time. Is there a guide or example?

@leezu
Copy link
Contributor

leezu commented Jan 22, 2021

@KaixuYang you can take a look at scikit-build https://scikit-build.readthedocs.io/en/latest/usage.html or "raw" pyproject.toml file: https://setuptools.readthedocs.io/en/latest/build_meta.html

Edit: I see sci-kit build still doesn't support PEP 517, PEP 518 scikit-build/scikit-build#124 though it should be possible to still use it

@KaixuYang
Copy link
Author

@leezu Is the goal to first install scikit-build in the setup.py file, then call it to include cmake? What are the dependencies that cmake is supposed to find automatically? Like the openblas or the other python dependencies?

@leezu
Copy link
Contributor

leezu commented Jan 23, 2021

cmake will find native dependencies, such as openblas and will error out if they can't be found. So you don't need to worry about them. Installation of sdist will only be supported if user has all native build dependencies installad. You can refer to https://mxnet.apache.org/versions/master/get_started/build_from_source.html for list of recommended dependencies

You use scikit-build in setup.py and declare that you need scikit-build in pyproject.toml. Then pip will ensure it's available when executing setup.py.

@KaixuYang
Copy link
Author

@leezu I'm not sure if I did it in the correct way, but I keep getting this error calling setup.py. What is the expected behavior of find_lib_path?

File "setup.py", line 47, in <module>
    LIB_PATH = libinfo['find_lib_path']()
  File "mxnet/libinfo.py", line 73, in find_lib_path
    raise RuntimeError('Cannot find the MXNet library.\n' +
RuntimeError: Cannot find the MXNet library.
List of candidates:
/Users/kaixuyang/mxnet/python/mxnet/libmxnet.dylib
/Users/kaixuyang/mxnet/python/mxnet/../../lib/libmxnet.dylib
/Users/kaixuyang/mxnet/python/mxnet/../../build/libmxnet.dylib
/Users/kaixuyang/mxnet/python/mxnet/libmxnet.so
/Users/kaixuyang/mxnet/python/mxnet/../../lib/libmxnet.so
/Users/kaixuyang/mxnet/python/mxnet/../../build/libmxnet.so

@KaixuYang
Copy link
Author

@leezu Where are the libmxnet.so and libmxnet.dylib files?

@leezu
Copy link
Contributor

leezu commented Jan 25, 2021

@leezu Where are the libmxnet.so and libmxnet.dylib files?

You need to setup the scikit-build to create these files with cmake

@KaixuYang
Copy link
Author

@leezu The build fails if cmake is not installed, is there a way to check this? (The brew install cmake, not the pip install cmake). Also how to turn off CUDA? The build checks for CUDA but it is not available.

@KaixuYang
Copy link
Author

@leezu This also happens with opencv.

@leezu
Copy link
Contributor

leezu commented Jan 25, 2021

@KaixuYang you can refer to https://github.com/apache/incubator-mxnet/blob/master/config/linux_gpu.cmake / https://github.com/apache/incubator-mxnet/blob/master/config/linux.cmake config files for a list of configuration options (including how to turn off cuda). For your initial development, I think you can hardcode this configuration option. Later you can try to auto-detect it.

@KaixuYang
Copy link
Author

@leezu Yeah I see these files. I'm wondering where does cmake learn which config (darwin.cmake, linux.cmake, linux_gpu.cmake) to use?

@KaixuYang
Copy link
Author

For example, when I don't have cuda on my machine, it should use the version without CUDA.

@leezu
Copy link
Contributor

leezu commented Jan 25, 2021

These configs only take effect if you copy them to the main folder (next to CMakeLists.txt) under the name config.cmake:

https://github.com/apache/incubator-mxnet/blob/1c3fcad78caa7e58f81045d6128435a59222dfdd/CMakeLists.txt#L28-L31

They are just for convenience. If the config is not present, the default values will be used

https://github.com/apache/incubator-mxnet/blob/1c3fcad78caa7e58f81045d6128435a59222dfdd/CMakeLists.txt#L39-L96

You can manually pass option to cmake via -DCONFIG_NAME (eg -DUSE_CUDA=1) to overwrite the default / config option.

@leezu
Copy link
Contributor

leezu commented Jan 25, 2021

So at this time you can either detect if cuda is present inside the setup.py and then pass -DUSE_CUDA=1 or 0, or you can refactor the CMakeLists.txt file to support a -DUSE_CUDA=Auto option (which would try to find cuda and disable cuda if it can't be found)

@KaixuYang
Copy link
Author

Yeah this makes sense, thanks!

@leezu
Copy link
Contributor

leezu commented Jan 25, 2021

Ok, thank you! For purpose of development, you can just set -DUSE_CUDA=0 inside setup.py for now (until you get that to work. Then take the next step).

@KaixuYang
Copy link
Author

@leezu seems like I got into another existing issue #16245

@leezu
Copy link
Contributor

leezu commented Jan 25, 2021

It will be fixed by #19610 For now you can delete this code or add an extra condition to ensure you take the else branch:

https://github.com/apache/incubator-mxnet/blob/1c3fcad78caa7e58f81045d6128435a59222dfdd/CMakeLists.txt#L415-L430

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants