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

Fix column major data layout in bindings #126

Merged
merged 2 commits into from
Aug 10, 2021
Merged

Fix column major data layout in bindings #126

merged 2 commits into from
Aug 10, 2021

Conversation

mlxd
Copy link
Member

@mlxd mlxd commented Aug 10, 2021

Context: This PR fixes issues observed when passing numpy data in column-major layout to the row-major backed C++ bindings.

Description of the Change: Numpy data is forced to C-style (row-major) layout when passing numpy arrays to the C++ backend.

Benefits: This ensures correctness for both row and column major data layouts.

Possible Drawbacks: Column major layout data will experience a small overhead (though we should be using row-major everything).

Related GitHub Issues: #124

@mlxd mlxd changed the title Fix column major data layout bindings Fix column major data layout in bindings Aug 10, 2021
@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@mlxd mlxd requested a review from trbromley August 10, 2021 13:48
@github-actions
Copy link
Contributor

github-actions bot commented Aug 10, 2021

Test Report (C++) on Ubuntu

    1 files  ±0      1 suites  ±0   0s ⏱️ ±0s
164 tests ±0  164 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
694 runs  ±0  694 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 14ea923. ± Comparison against base commit 14ea923.

♻️ This comment has been updated with latest results.

@codecov
Copy link

codecov bot commented Aug 10, 2021

Codecov Report

Merging #126 (aa8102f) into master (abf7b99) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #126   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines           54        54           
=========================================
  Hits            54        54           

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 abf7b99...aa8102f. Read the comment docs.

@mlxd
Copy link
Member Author

mlxd commented Aug 10, 2021

[ch8073]

Copy link
Contributor

@trbromley trbromley left a comment

Choose a reason for hiding this comment

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

💯

@mlxd mlxd merged commit 14ea923 into master Aug 10, 2021
@mlxd mlxd deleted the 8073_numpy_colmajor branch August 10, 2021 14:04
antalszava added a commit that referenced this pull request Aug 11, 2021
antalszava added a commit that referenced this pull request Aug 11, 2021
antalszava added a commit that referenced this pull request Aug 11, 2021
* Update method signatures for bound class (#125)

* Add utility to get qubit number

* Define gate overloads to enable run-time generalised calls

* Update method signatures

* Update changelog

* Fix column major data layout in bindings (#126)

* Fix column major data layout bindings

* Update changelog

* increment version, update changelog, gitignore a build folder

* no Python version 3.6

* changelog for removing Python 3.6

* Revert "Fix column major data layout in bindings (#126)"

This reverts commit 14ea923.

* resolve changelog conflict

* Update .github/CHANGELOG.md

* Revert "Revert "Fix column major data layout in bindings (#126)""

This reverts commit 1de97e6.

Co-authored-by: Lee James O'Riordan <[email protected]>
Co-authored-by: Antal Szava <[email protected]>
antalszava added a commit that referenced this pull request Aug 17, 2021
* V17.0 increment version (#127)

* Update method signatures for bound class (#125)

* Add utility to get qubit number

* Define gate overloads to enable run-time generalised calls

* Update method signatures

* Update changelog

* Fix column major data layout in bindings (#126)

* Fix column major data layout bindings

* Update changelog

* increment version, update changelog, gitignore a build folder

* no Python version 3.6

* changelog for removing Python 3.6

* Revert "Fix column major data layout in bindings (#126)"

This reverts commit 14ea923.

* resolve changelog conflict

* Update .github/CHANGELOG.md

* Revert "Revert "Fix column major data layout in bindings (#126)""

This reverts commit 1de97e6.

Co-authored-by: Lee James O'Riordan <[email protected]>
Co-authored-by: Antal Szava <[email protected]>

* organise changelog

* version bump

Co-authored-by: Christina Lee <[email protected]>
Co-authored-by: Lee James O'Riordan <[email protected]>
mlxd added a commit that referenced this pull request Aug 20, 2021
* V17.0 increment version (#127)

* Update method signatures for bound class (#125)

* Add utility to get qubit number

* Define gate overloads to enable run-time generalised calls

* Update method signatures

* Update changelog

* Fix column major data layout in bindings (#126)

* Fix column major data layout bindings

* Update changelog

* increment version, update changelog, gitignore a build folder

* no Python version 3.6

* changelog for removing Python 3.6

* Revert "Fix column major data layout in bindings (#126)"

This reverts commit 14ea923.

* resolve changelog conflict

* Update .github/CHANGELOG.md

* Revert "Revert "Fix column major data layout in bindings (#126)""

This reverts commit 1de97e6.

Co-authored-by: Lee James O'Riordan <[email protected]>
Co-authored-by: Antal Szava <[email protected]>

* Update documentation for Python bindings

* Add docstrings to gate matrix definitions

* Add docstrings for all publically facing statevector methods

* Fix incorrectly labelled gatecalls

* Update doxygen comments to ignore anon namespaces

* Move exc into namespace

* Update missing gates

* Enable build of C++ API with docs

* Add C++ API to docs

* Add C4 arch diagrams for lightning

* Add sample C4 architecture for lightning using current design

* Fix missing packages

* Update documentation architecture

* Prevent bindings from generating API docs

* Update changelog

Co-authored-by: Christina Lee <[email protected]>
Co-authored-by: Antal Szava <[email protected]>
Co-authored-by: Tom Bromley <[email protected]>
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.

2 participants