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

Use Python 3.9 [in public CI] #599

Merged
merged 7 commits into from
Sep 30, 2021
Merged

Use Python 3.9 [in public CI] #599

merged 7 commits into from
Sep 30, 2021

Conversation

PokhodenkoSA
Copy link
Contributor

@PokhodenkoSA PokhodenkoSA commented Sep 28, 2021

  • Update CI
  • Update conda recipe
  • Use intel channel

@coveralls
Copy link
Collaborator

coveralls commented Sep 28, 2021

Coverage Status

Coverage remained the same at 74.162% when pulling d1ffeb6 on spokhode/enh/py39 into b180d59 on master.

@PokhodenkoSA
Copy link
Contributor Author

I had to disable intel channel for building because of CMake compiler check fail.
It should be investigated.

@oleksandr-pavlyk
Copy link
Collaborator

Is it perhaps better to restrict the version of cmake than to remove the intel channel. We need to be sure we are picking correct run time bits.

@PokhodenkoSA
Copy link
Contributor Author

PokhodenkoSA commented Sep 28, 2021

Is it perhaps better to restrict the version of cmake than to remove the intel channel. We need to be sure we are picking correct run time bits.

Thank for the idea. I will check that the problem is really in cmake version.

@PokhodenkoSA
Copy link
Contributor Author

Problem is not related to CMake.
I compared py38 build log and py39. In py38 dpcpp_cpp_rt is installed in host environment. It contains libintlc.so.5 which is required by clang we use for building. Actually, dpcpp_cpp_rt is installed in build environment during building but for some reason it has no effect.
In py38 dpcpp_cpp_rt is installed in host environment because it is installed with other packages like mkl and numpy from intel channel. Now this packages are not provided by intel channel for py39 and so they are installed from defaults. Packages on defaults channel does not depend on dpcpp_cpp_rt.

Copy link
Contributor

@AndresGuzman-Ballen AndresGuzman-Ballen left a comment

Choose a reason for hiding this comment

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

LGTM!

@oleksandr-pavlyk
Copy link
Collaborator

@AndresGuzman-Ballen Why isn't the compiler package carrying the run-time? If run-time is not present, how can it be a viable compiler package?

@AndresGuzman-Ballen
Copy link
Contributor

@oleksandr-pavlyk Sergey already answered why this is appearing in Python 3.9 and not Python 3.8. What I imagine is happening is that dpctl is expecting the runtimes to be found in the host prefix but since {{ compiler('dpcpp') }} is a build dependency, it is being installed in the build prefix instead

@oleksandr-pavlyk
Copy link
Collaborator

@AndresGuzman-Ballen Per @PokhodenkoSA's explanation, it is the compiler that requires intlc library. The compiler is configured by cmake, and so I suspect there may be an issue with how Python driver sets paths for cmake.

dpctl Python extensions should not be built using dpcpp/clang at present, only the backend library.

I am trying to build the recipe locally.

@oleksandr-pavlyk
Copy link
Collaborator

The build environment is indeed viable, as once I activate it, I am able to build a simple C++ program that uses SYCL. This means that build environment already contains all the components in dpcpp_cpp_rt.

DPC++ launched by cmake does not see those components, because conda build isolates LD_LIBRARY_PATH to only $PREFIX subfolders, specifically: LD_LIBRARY_PATH=$PREFIX/compiler/lib/intel64_lin:$PREFIX/compiler/lib:$PREFIX/lib:.

I was able to build Python 3.9 package using conda build with the following set of changes atop the tip of master branch:

diff --git a/conda-recipe/build.sh b/conda-recipe/build.sh
index 37ae591..f2df115 100755
--- a/conda-recipe/build.sh
+++ b/conda-recipe/build.sh
@@ -3,10 +3,11 @@
 # Workaround to Klocwork overwriting LD_LIBRARY_PATH that was modified
 # by DPC++ compiler conda packages. Will need to be added to DPC++ compiler
 # activation scripts.
-export LDFLAGS="$LDFLAGS -Wl,-rpath,$CONDA_PREFIX/lib"
+export LDFLAGS="$LDFLAGS -Wl,-rpath,$PREFIX/lib"

 ${PYTHON} setup.py clean --all
-INSTALL_CMD="install --sycl-compiler-prefix=$CONDA_PREFIX"
+INSTALL_CMD="install --sycl-compiler-prefix=$BUILD_PREFIX"
+export LD_LIBRARY_PATH=$BUILD_PREFIX/lib:${LD_LIBRARY_PATH}

 if [ -n "${WHEELS_OUTPUT_FOLDER}" ]; then
     # Install packages and assemble wheel package from built bits
diff --git a/conda-recipe/meta.yaml b/conda-recipe/meta.yaml
index 77c7814..ce4da0b 100644
--- a/conda-recipe/meta.yaml
+++ b/conda-recipe/meta.yaml
@@ -22,7 +22,7 @@ requirements:
         - python
         - make  # [unix]
         - ninja  # [win]
-        - numpy >=1.17 # [win or osx or py==38]
+        - numpy >=1.17 # [win or osx or py>=38]
         - numpy 1.17 # [linux and py==37]
         - wheel
     run:

I used standard conda build --no-test --python 3.9 -c intel -c defaults --override-channels conda-recipe.

@PokhodenkoSA
Copy link
Contributor Author

PokhodenkoSA commented Sep 30, 2021

I have applied Sasha's changes. LGTM.
I will merge this PR as I need it for continuing work for other packages.
If you need other improvements, please, open another PR.

@PokhodenkoSA
Copy link
Contributor Author

I have made job "Generate coverage data / Generate coverage and push to Coveralls.io" as not required because it fails on master too.

Copy link

@Alexander-Makaryev Alexander-Makaryev left a comment

Choose a reason for hiding this comment

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

LGTM

@PokhodenkoSA PokhodenkoSA merged commit 6d005b2 into master Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure py39 Python version specific issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CMake compiler check fail with Python 3.9 and intel channel
5 participants