[WIP] setup.py uses CMake #176

chaeyeunpark · 2021-11-12T15:41:00Z

setup.py is rewritten to use CMake.
Compiling guide for MSVC is added (README.rst).

…htning into cmake_setup_py

codecov · 2021-11-12T15:43:12Z

Codecov Report

Merging #176 (7da1e65) into master (5c19079) will decrease coverage by 7.02%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##            master     #176      +/-   ##
===========================================
- Coverage   100.00%   92.97%   -7.03%     
===========================================
  Files            4        4              
  Lines          184      185       +1     
===========================================
- Hits           184      172      -12     
- Misses           0       13      +13

Impacted Files	Coverage Δ
pennylane_lightning/lightning_qubit.py	`88.88% <100.00%> (-11.12%)`	⬇️
pennylane_lightning/_serialize.py	`98.61% <0.00%> (-1.39%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c19079...7da1e65. Read the comment docs.

github-actions · 2021-11-12T15:43:56Z

Test Report (C++) on Ubuntu

      1 files ±0       1 suites ±0 0s ⏱️ ±0s
  345 tests ±0   345 ✔️ ±0 0 💤 ±0 0 ❌ ±0
1 919 runs ±0 1 919 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit 7da1e65. ± Comparison against base commit 5c19079.

♻️ This comment has been updated with latest results.

chaeyeunpark · 2021-11-12T21:40:18Z

Windows tests fail as python does not find a correct DLL path. https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew can be a reason?

mlxd · 2021-11-16T09:07:22Z

It may also be worth enabling the CI wheel builders to export the built wheels for this PR (they are normally disabled outside master). As a quick workaround, we can remove this line:

pennylane-lightning/.github/workflows/wheel_linux_x86_64.yml

Line 57 in 63929ed

if: ${{ github.event_name == 'release' || github.ref == 'refs/heads/master' }}

and re-add it after we pull down the wheel. Same as for Mac x86_64 and M1 (to verify OpenMP no longer fails).

chaeyeunpark · 2021-11-16T16:14:17Z

It may also be worth enabling the CI wheel builders to export the built wheels for this PR (they are normally disabled outside master). As a quick workaround, we can remove this line:

pennylane-lightning/.github/workflows/wheel_linux_x86_64.yml

Line 57 in 63929ed

if: ${{ github.event_name == 'release' || github.ref == 'refs/heads/master' }}

and re-add it after we pull down the wheel. Same as for Mac x86_64 and M1 (to verify OpenMP no longer fails).

See https://github.com/PennyLaneAI/pennylane-lightning/actions/runs/1467603819 and https://github.com/PennyLaneAI/pennylane-lightning/actions/runs/1467603816

mlxd · 2021-11-22T11:37:42Z

Hi @chaeyeunpark I had time to compare the wheels on Python3.9 for Linux. The performance is pretty much the same for both (nice work on that). I did notice that the size of the wheels, as well as the exported symbols, differ between the current version and this version. I think for now to minimize the differences we can add some additional compile-time flags that are turned on by-default for the Linux/Mac setuptools versions:

## Allows overflows of integer ops; should never be a problem and can potentially add some improvements
-fwrapv

## Allows more optimal external library calls
-fno-plt

## Avoid intermediate files hitting the disk; speeds up compilation in some cases
-pipe

I think this should be enough to get an almost apples-to-apples binary comparison. With the above changes, and disabling the wheels being stored, I am happy to see this being merged in.

I suspect the CodeCov issue will resolve itself too with another commit. If not, we can see about whether it makes sense to add another test, or just override its complaint.

chaeyeunpark · 2021-11-22T22:59:08Z

Hi @mlxd, I made those changes https://github.com/PennyLaneAI/pennylane-lightning/pull/176/files#diff-f8588b38421f6ebc8fdb5eaaca2d2a160b4e38a5092a2ffa045f7cd5075823eaR10-R17. However, tests still fail (probably because of the new Pennylane features). We may merge this after #179 is merged.

mlxd · 2021-11-23T09:29:37Z

Hi @mlxd, I made those changes https://github.com/PennyLaneAI/pennylane-lightning/pull/176/files#diff-f8588b38421f6ebc8fdb5eaaca2d2a160b4e38a5092a2ffa045f7cd5075823eaR10-R17. However, tests still fail (probably because of the new Pennylane features). We may merge this after #179 is merged.

Awesome work! Thanks @chaeyeunpark

Fixed --build options

maliasadi · 2021-11-23T18:42:58Z

We may merge this after #179 is merged.

Nice work @chaeyeunpark! PR #179 is also merged.

Updated install guide for development setting.

mlxd

Hey Chae-Yeun, I added two more comments. Happy to merge after these are addressed.

mlxd · 2021-11-24T16:05:45Z

.github/workflows/wheel_noarch.yml

@@ -19,6 +19,9 @@ jobs:
        with:
          python-version: '3.7'

+      - name: Get latest CMake and ninja
+        uses: lukka/get-cmake@latest


Is there any reason to prefer this over a pip installation of both cmake and ninja?

mlxd · 2021-11-24T16:07:31Z

pennylane_lightning/lightning_qubit.py

+    if platform.system() == "Windows" and sys.version_info[:2] >= (3, 8):
+        # Add the current directory to DLL path.
+        # See https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew
+        os.add_dll_directory(os.path.dirname(os.path.abspath(__file__)))


Maybe we can add a Pylint ignore comment here to ignore the warnings. Or, should we add tests as recommended?

This reverts commit 19a8a85.

chaeyeunpark and others added 17 commits November 5, 2021 21:46

Rewrite the simplest setup.py for CMake

ff5e211

Rewrite the simplest setup.py for CMake

b034233

Merge branch 'cmake_setup_py' of github.com:PennyLaneAI/pennylane-lig…

b9685af

…htning into cmake_setup_py

Cut clutter in CMakeLists.txt

b0d50ed

setup.py correctly configure --define

e84ddbb

Refactored CMakeLists.txt

d08b5b1

OpenMP fixed

0ea2819

Now uses Ninja

9785770

Disable OpenMP ans BLAS for Windows

db4e2ca

Corrected BLAS message

e338cc1

Suppress warnings for release mode in MSVC

8720386

Ninja version relaxed

6d6d558

Format fixed

52f53a8

Merge branch 'master' into cmake_setup_py

75d89da

Now path is str

5c7cb7b

More path is str

86ef04d

Test with explicit cmake

df129fd

Configureations for cibuildwheel updated

23b61fe

chaeyeunpark force-pushed the cmake_setup_py_actions_wheel branch from e4b68f0 to 23b61fe Compare November 12, 2021 19:40

chaeyeunpark added 3 commits November 12, 2021 14:49

Updated commands for windows

9f7b892

Only target AMD64 for Windows

4ba3e20

Should call vcvars64.bat before test

abad7f6

chaeyeunpark added 5 commits November 12, 2021 16:50

Test python path for Windows

6283fa9

Removed comments

9d11e35

Adds dependencies to readthedocs

0b51693

Command fixed

cc01c06

Fixed

e6d9870

chaeyeunpark added 4 commits November 16, 2021 09:53

Disable OpenMP in M1 Macs; Test upload wheel

9b92ecb

Merge branch 'master' into cmake_setup_py_actions_wheel

034c460

Test for M1

6d60ff2

Must disable OpenMP instead of BLAS

0a90da2

chaeyeunpark requested a review from mlxd November 16, 2021 16:17

chaeyeunpark and others added 3 commits November 22, 2021 17:30

Update README.rst

5dca7c9

Do not upload wheel for branchs; Added some compile options

c416e3f

Corrected comment

91f04ec

Update README.rst

43e7c63

Fixed --build options

chaeyeunpark and others added 4 commits November 23, 2021 13:47

README.rst fixed; BLAS option fixed

d50f69c

Merge branch 'master' into cmake_setup_py_actions_wheel

c6395ed

Update README.rst

8855b6c

Updated install guide for development setting.

Update README.rst

abd894b

mlxd reviewed Nov 24, 2021

View reviewed changes

chaeyeunpark added 3 commits November 25, 2021 09:42

Merge branch 'master' into cmake_setup_py_actions_wheel

10addaf

Updated noarch; Added nolint for coverage

0fe149a

Do not upload wheel for noarch if not master; format-fixed

f981070

mlxd self-requested a review November 25, 2021 17:55

mlxd approved these changes Nov 25, 2021

View reviewed changes

Wheel script for noarch fixed

7da1e65

chaeyeunpark merged commit 19a8a85 into master Nov 25, 2021

chaeyeunpark deleted the cmake_setup_py_actions_wheel branch November 25, 2021 18:57

chaeyeunpark added a commit that referenced this pull request Nov 25, 2021

Revert "[WIP] setup.py uses CMake (#176)"

57e3386

This reverts commit 19a8a85.

chaeyeunpark mentioned this pull request Feb 4, 2022

Include C++ source files in binary wheels #225

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] setup.py uses CMake #176

[WIP] setup.py uses CMake #176

chaeyeunpark commented Nov 12, 2021 •

edited

Loading

codecov bot commented Nov 12, 2021 •

edited

Loading

github-actions bot commented Nov 12, 2021 •

edited

Loading

chaeyeunpark commented Nov 12, 2021

mlxd commented Nov 16, 2021 •

edited

Loading

chaeyeunpark commented Nov 16, 2021

mlxd commented Nov 22, 2021

chaeyeunpark commented Nov 22, 2021

mlxd commented Nov 23, 2021

maliasadi commented Nov 23, 2021

mlxd left a comment •

edited

Loading

mlxd Nov 24, 2021

mlxd Nov 24, 2021

[WIP] setup.py uses CMake #176

[WIP] setup.py uses CMake #176

Conversation

chaeyeunpark commented Nov 12, 2021 • edited Loading

codecov bot commented Nov 12, 2021 • edited Loading

Codecov Report

github-actions bot commented Nov 12, 2021 • edited Loading

Test Report (C++) on Ubuntu

chaeyeunpark commented Nov 12, 2021

mlxd commented Nov 16, 2021 • edited Loading

chaeyeunpark commented Nov 16, 2021

mlxd commented Nov 22, 2021

chaeyeunpark commented Nov 22, 2021

mlxd commented Nov 23, 2021

maliasadi commented Nov 23, 2021

mlxd left a comment • edited Loading

Choose a reason for hiding this comment

mlxd Nov 24, 2021

Choose a reason for hiding this comment

mlxd Nov 24, 2021

Choose a reason for hiding this comment

chaeyeunpark commented Nov 12, 2021 •

edited

Loading

codecov bot commented Nov 12, 2021 •

edited

Loading

github-actions bot commented Nov 12, 2021 •

edited

Loading

mlxd commented Nov 16, 2021 •

edited

Loading

mlxd left a comment •

edited

Loading