Skip to content
This repository was archived by the owner on Feb 2, 2024. It is now read-only.

Conversation

@Vyacheslav-Smirnov
Copy link
Contributor

@Vyacheslav-Smirnov Vyacheslav-Smirnov commented Feb 3, 2020

Build instruction in Readme.rst is not reflect to current SDC build process.

  • Add step of specific commit Numba build
  • Correct packages versions
  • Add missed packages required for Numba build

README.rst Outdated
PYVER=<3.6 or 3.7>
conda create -n SDC -q -y -c numba -c defaults -c conda-forge python=$PYVER numba pandas scipy mpich pyarrow=0.15.1 gcc_linux-64 gxx_linux-64
NUMPYVER=<1.16 or 1.17>
conda create -n SDC -q -y -c numba -c defaults -c intel -c conda-forge python=$PYVER numpy=$NUMPYVER pandas=0.25.3 scipy pyarrow=0.15.1 gcc_linux-64 gxx_linux-64 tbb-devel llvmlite=0.31.0rc1=py*_0
Copy link
Contributor

Choose a reason for hiding this comment

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

llvmlite is a dependency of numba. Why not remove it from the list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need specific llvmlite version for Numba

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also here we build numba via setuptools thus need to install its dependencies

Copy link
Contributor

Choose a reason for hiding this comment

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

When we will update Numba version we also will have to check new dependencies and modify this line. Is there more convenient way to install numba dependencies?

Also, I think we now could use newer version of llvmlite.

README.rst Outdated
# Build Numba
git clone https://github.com/numpy/numpy.git
cd numba
git checkout ef119bcd1733ff49d71bdf2da8a66e91bb704f83
Copy link
Contributor

Choose a reason for hiding this comment

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

This magic constant is commit number. Could you, please, create variable with name explaining what is specific with this commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do not think it is required to create a variable for checkouting Numba hash
I just mentioned that
"Intel SDC uses Numba ef119bcd1733ff49d71bdf2da8a66e91bb704f83 commit from master branch as Numba package for build and run.
That is why it is required to build specified Numba first. Build steps are described below."
Do you think there should be described features which we are using?

Copy link
Contributor

Choose a reason for hiding this comment

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

This commit is mentioned 3 times. And all of them should be updated after migration to the new one.

README.rst Outdated
# Build Numba
git clone https://github.com/numpy/numpy.git
cd numba
git checkout ef119bcd1733ff49d71bdf2da8a66e91bb704f83
Copy link
Contributor

Choose a reason for hiding this comment

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

This commit is mentioned 3 times. And all of them should be updated after migration to the new one.

README.rst Outdated
PYVER=<3.6 or 3.7>
conda create -n SDC -q -y -c numba -c defaults -c conda-forge python=$PYVER numba pandas scipy mpich pyarrow=0.15.1 gcc_linux-64 gxx_linux-64
NUMPYVER=<1.16 or 1.17>
conda create -n SDC -q -y -c numba -c defaults -c intel -c conda-forge python=$PYVER numpy=$NUMPYVER pandas=0.25.3 scipy pyarrow=0.15.1 gcc_linux-64 gxx_linux-64 tbb-devel llvmlite=0.31.0rc1=py*_0
Copy link
Contributor

Choose a reason for hiding this comment

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

When we will update Numba version we also will have to check new dependencies and modify this line. Is there more convenient way to install numba dependencies?

Also, I think we now could use newer version of llvmlite.

@Vyacheslav-Smirnov
Copy link
Contributor Author

When we updating Numba we should always check the documentation for update.
I just want to make it more clear and relevant to current situation - in Numba recipe it is defined llvmlite=0.31.0, thus the user who wants to build Numba using setuptools should install the same llvmlite (for example, conda resolver may install less or more version if version is not specified and Numba can be failed to build)

@AlexanderKalistratov
Copy link
Collaborator

@PokhodenkoSA ping

@PokhodenkoSA
Copy link
Contributor

@Vyacheslav-Smirnov could you please describe improvements of this PR in PR description? It is not clear for me what is the reason of this PR.

@Vyacheslav-Smirnov
Copy link
Contributor Author

@PokhodenkoSA, PR description updated.

@PokhodenkoSA PokhodenkoSA merged commit 7150b62 into IntelPython:master Feb 18, 2020
@Vyacheslav-Smirnov Vyacheslav-Smirnov deleted the update_build_doc branch February 20, 2020 10:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants