Skip to content

CI: Intel icx/icpx via oneAPI #2769

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

CI: Intel icx/icpx via oneAPI #2769

wants to merge 1 commit into from

Conversation

ax3l
Copy link
Collaborator

@ax3l ax3l commented Jan 6, 2021

Add testing for Intel icx/icpx via the oneAPI images.
This is Intel's next-gen compiler and new C++ support, e.g. C++17 bug fixes, first land here.

For the Intel "classic" compilers icc/icpc, please see #2573

@YannickJadoul
Copy link
Collaborator

YannickJadoul commented Jan 7, 2021

New compiler, all green, within 24h? Is this possible, or am I dreaming?

EDIT: Oh... no. Somehow, the CI workflow hasn't even run :-(

@henryiii henryiii closed this Jan 7, 2021
@henryiii henryiii reopened this Jan 7, 2021
@henryiii
Copy link
Collaborator

henryiii commented Jan 7, 2021

Yaml must be invalid?

@henryiii henryiii added the compiler: intel Related to the Intel compilers label Jan 7, 2021
@ax3l ax3l force-pushed the topic-icpx branch 4 times, most recently from c76931a to eb1c683 Compare January 8, 2021 19:24
@henryiii
Copy link
Collaborator

@ax3l The issue now is in the CMake config to force it to recognize icpx.

Copy link
Collaborator Author

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

Potential these two are also needed:
Fix compiler detection that made it into the CMake shipped with oneAPI. This is already fixed in CMake upstream.

@ax3l ax3l force-pushed the topic-icpx branch 5 times, most recently from c8c0708 to b6d72e1 Compare January 16, 2021 00:40
-DCMAKE_CXX_COMPILER_VERSION=12.0 \
-DCMAKE_CXX_STANDARD_COMPUTED_DEFAULT="14" \
-DCMAKE_VERBOSE_MAKEFILE=ON \
-DPYTHON_EXECUTABLE=$(python3 -c "import sys; print(sys.executable)")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is this actually preferred over $(which python3)?

Copy link
Collaborator

@henryiii henryiii Jan 16, 2021

Choose a reason for hiding this comment

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

This works on bash for Windows or something like that, I don't remember exactly why. Pretty sure it had to do with portability somehow. which should be just fine for this sort of thing on linux for sure.

@henryiii
Copy link
Collaborator

I am making vectorize a bit easier to compile in #2729 - don't know why it's compiling but not working here.

@henryiii
Copy link
Collaborator

henryiii commented Jan 16, 2021

What is this identifying as, I wonder? Does it define the intel macros? Clang macros? Both? Neither?

@ax3l
Copy link
Collaborator Author

ax3l commented Feb 9, 2021

What is this identifying as, I wonder? Does it define the intel macros? Clang macros? Both? Neither?

Oh that's a bug that slipped into CMake v3.19.0-rc1-v3.19.1 (reverted afterwards) and that in turn slipped into a oneAPI release: https://gitlab.kitware.com/cmake/cmake/-/issues/21551

It's reverted now and complete IntelLLVM and SYCL/DPC++ support is being worked on:

So if we leave this open until the next oneAPI release comes out and ships CMake 3.20+ (usually a month max.) then we should be able to remove the work-around of the compiler detection.

@ax3l ax3l marked this pull request as draft February 9, 2021 19:34
@ax3l
Copy link
Collaborator Author

ax3l commented Feb 12, 2021

Once we have IntelLLVM (icx) in, we can start to explore the reason for those multiple-definition errors of math functions with dpc++ (aka icx --dpcpp -fsycl).
AMReX-Codes/pyamrex#5

Likely an upstream Intel issue, but we'll see.

@henryiii henryiii force-pushed the topic-icpx branch 2 times, most recently from 4330516 to c2512ee Compare August 21, 2021 16:43
@ax3l ax3l force-pushed the topic-icpx branch 2 times, most recently from 6788039 to 9ef16dc Compare January 4, 2022 12:48
Add testing for Intel icx/icpx via the oneAPI images.
This is Intel's next-gen compiler and new C++ support, e.g. C++17 bug
fixes, first land here.

Update .github/workflows/ci.yml

Co-authored-by: Yannick Jadoul <[email protected]>

CMake: Fix ICPX Detection in Python tests

More IntelClang Detection

Upsi, keep ICC

rebased out by accident

Prettify

Compiler Workaround: Just in Configure

ci: rely on newer CMake
@ax3l
Copy link
Collaborator Author

ax3l commented Jan 4, 2022

Yay, made some progress. Now we have a few tests that fail the vectorization tests.

test_vectorize

________________________________ test_vectorize ________________________________

    def test_vectorize():
        n = 3
        array = m.create_rec_simple(n)
        values = m.f_simple_vectorized(array)
>       np.testing.assert_array_equal(values, [0, 10, 20])
E       AssertionError: 
E       Arrays are not equal
E       
E       Mismatched elements: 2 / 3 (66.7%)
E       Max absolute difference: 20
E       Max relative difference: 1.
E        x: array([0, 0, 0], dtype=uint32)
E        y: array([ 0, 10, 20])

../../tests/test_numpy_dtypes.py:375: AssertionError

test_vectorized_noreturn

___________________________ test_vectorized_noreturn ___________________________

    def test_vectorized_noreturn():
        x = m.NonPODClass(0)
        assert x.value == 0
        m.add_to(x, [1, 2, 3, 4])
>       assert x.value == 10
E       assert 4 == 10
E        +  where 4 = <pybind11_tests.numpy_vectorize.NonPODClass object at 0x7f52c5d3f730>.value

../../tests/test_numpy_vectorize.py:263: AssertionError

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler: intel Related to the Intel compilers enhancement help wanted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants