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

Include Kokkos & OpenQASM backend in binary distribution #198

Merged
merged 4 commits into from
Jul 13, 2023

Conversation

dime10
Copy link
Collaborator

@dime10 dime10 commented Jul 12, 2023

No description provided.

@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #198 (41aafc9) into v0.2.1-rc (afa7c09) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           v0.2.1-rc     #198   +/-   ##
==========================================
  Coverage      99.09%   99.09%           
==========================================
  Files             37       37           
  Lines           6440     6440           
  Branches         331      331           
==========================================
  Hits            6382     6382           
  Misses            33       33           
  Partials          25       25           
Impacted Files Coverage Δ
frontend/catalyst/_version.py 100.00% <100.00%> (ø)

@dime10 dime10 requested a review from maliasadi July 12, 2023 22:06
@@ -75,6 +75,8 @@ jobs:
QIR_STDLIB_DIR="$(pwd)/qir-stdlib-build" \
QIR_STDLIB_INCLUDES_DIR="$(pwd)/qir-stdlib-build/include" \
ENABLE_LIGHTNING_KOKKOS=ON \
CMAKE_ARGS="-DKokkos_ENABLE_OPENMP=ON" \
ENABLE_OPENQASM=ON \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I also included the OpenQASM backend here in the runtime build job, rather than further down in the OQ3 test job, since that's how we do it for kokkos as well. Or was there a specific reason to rebuild the runtime in the test job? :)

Copy link
Member

Choose a reason for hiding this comment

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

The build with OQ3 is not sufficient as of its dependency to the Amazon Braket Python SDK. I didn't want to end up building something that's partially working in this GH job, so ended up building the runtime in the OQ3 test job. Also, building runtime is quite fast with or without this backend, so there wasn't any remarkable changes to the CI total time.

doc/changelog.md Outdated Show resolved Hide resolved
@@ -333,3 +336,5 @@ jobs:
- name: Run Python Pytest Tests
run: |
python${{ matrix.python_version }} -m pytest $GITHUB_WORKSPACE/frontend/test/pytest -n auto
python${{ matrix.python_version }} -m pytest $GITHUB_WORKSPACE/frontend/test/pytest --backend="lightning.kokkos" -n auto
python${{ matrix.python_version }} -m pytest $GITHUB_WORKSPACE/frontend/test/pytest --runbraket=ON -k "not remotetests" -n auto
Copy link
Collaborator Author

@dime10 dime10 Jul 12, 2023

Choose a reason for hiding this comment

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

I guess the remote tests will trigger a failure here for this test (I had to comment them locally), until we get your changes in @maliasadi. Alternatively we could also add the credential setup here like we do in the check-catalyst action.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, this is tackled in my PR

.github/workflows/check-catalyst.yaml Show resolved Hide resolved
@@ -75,6 +75,8 @@ jobs:
QIR_STDLIB_DIR="$(pwd)/qir-stdlib-build" \
QIR_STDLIB_INCLUDES_DIR="$(pwd)/qir-stdlib-build/include" \
ENABLE_LIGHTNING_KOKKOS=ON \
CMAKE_ARGS="-DKokkos_ENABLE_OPENMP=ON" \
ENABLE_OPENQASM=ON \
Copy link
Member

Choose a reason for hiding this comment

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

The build with OQ3 is not sufficient as of its dependency to the Amazon Braket Python SDK. I didn't want to end up building something that's partially working in this GH job, so ended up building the runtime in the OQ3 test job. Also, building runtime is quite fast with or without this backend, so there wasn't any remarkable changes to the CI total time.

.github/workflows/check-catalyst.yaml Show resolved Hide resolved

<h3>Bug fixes</h3>

* Add missing OpenQASM backend in binary distribution, which relies on the latest version of the
AWS Braket plugin for PennyLane to resolve dependency issues between the plugin, Catalyst, and
PennyLane. The Lightning-Kokkos backend with Serial and OpenMP modes is also added to the binary
Copy link
Member

@maliasadi maliasadi Jul 13, 2023

Choose a reason for hiding this comment

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

We're not building against the SERIAL kernels anymore.

Suggested change
PennyLane. The Lightning-Kokkos backend with Serial and OpenMP modes is also added to the binary
PennyLane. The Lightning-Kokkos backend with OpenMP kernels is also added to the binary

@@ -333,3 +336,5 @@ jobs:
- name: Run Python Pytest Tests
run: |
python${{ matrix.python_version }} -m pytest $GITHUB_WORKSPACE/frontend/test/pytest -n auto
python${{ matrix.python_version }} -m pytest $GITHUB_WORKSPACE/frontend/test/pytest --backend="lightning.kokkos" -n auto
python${{ matrix.python_version }} -m pytest $GITHUB_WORKSPACE/frontend/test/pytest --runbraket=ON -k "not remotetests" -n auto
Copy link
Member

Choose a reason for hiding this comment

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

Yep, this is tackled in my PR

@dime10 dime10 merged commit 0111456 into v0.2.1-rc Jul 13, 2023
18 checks passed
@dime10 dime10 deleted the dime10-patch-0 branch July 13, 2023 07:21
dime10 added a commit that referenced this pull request Jul 17, 2023
* Include Kokkos & OpenQASM backend in binary distribution (#198)

* Include Kokkos & OQ3 backend in wheel

* Build OpenMP Kokkos in CI check

* Don't explicitly rebuild runtime for OQ tests

* Update doc/changelog.md

* Update the Braket/OQ3 support (#199)

* Update the support

* Update changelog

* Fix issue with pybind not being found

---------

Co-authored-by: David Ittah <dime10@users.noreply.github.com>

* Fix wheel building for the new OpenQASM component (#200)

* Improve organization in wheel action

E.g. runtime builds first to catch errors before lengthy MLIR build.

* Disable Python module build for Lightning backend

* Enable SERIAL Kokkos backend on top of OpenMP

* Try to fix Python / Pybind path discovery in cmake

* Make the Python static lib discoverable

* Revert "Make the Python static lib discoverable"

This reverts commit 0275adc.

* Don't embed Python when using Pybind (runtime)

This requires that the generated shared lib be loaded from Python,
or that the Python interpreter library be linked against the executable
compiled against the generated shared lib.

* To avoid link to libpython, use pybind11::module interface lib

* Disable python bindings in Lightning and Lightning-Kokkos

---------

Co-authored-by: Ali Asadi <ali@xanadu.ai>

* Install PL Plugins for LKokkos and Braket/OQ3 backend devices

* Upate changelog (#205)

* Update frontend/catalyst/_version.py

---------

Co-authored-by: Ali Asadi <ali@xanadu.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants