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

Add Harmony backend for IonQ #49

Closed
wants to merge 4 commits into from

Conversation

Cynocracy
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Mar 28, 2022

Codecov Report

Merging #49 (53ea8cb) into master (f69916a) will decrease coverage by 3.25%.
The diff coverage is 84.61%.

@@            Coverage Diff             @@
##           master      #49      +/-   ##
==========================================
- Coverage   88.77%   85.51%   -3.26%     
==========================================
  Files           5        5              
  Lines         285      290       +5     
==========================================
- Hits          253      248       -5     
- Misses         32       42      +10     
Impacted Files Coverage Δ
pennylane_ionq/device.py 89.90% <75.00%> (-3.37%) ⬇️
pennylane_ionq/__init__.py 100.00% <100.00%> (ø)
pennylane_ionq/api_client.py 91.19% <100.00%> (-3.78%) ⬇️

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 f69916a...53ea8cb. Read the comment docs.

@dime10 dime10 requested review from albi3ro and removed request for albi3ro March 28, 2022 23:21
@albi3ro
Copy link
Contributor

albi3ro commented Mar 31, 2022

Hi @Cynocracy and thanks for opening a PR.

Could you provide some information about what you are wanting to achieve with this PR?

You may also need to upgrade your version of black and use the -l 100 line length specification. In the most recent version, the formatting of a ** b changed to a**b.

@Cynocracy
Copy link
Contributor Author

Thank you! Will update and do another formatting pass

@Cynocracy
Copy link
Contributor Author

Also Hi :) Nice to meet you!

@Cynocracy Cynocracy marked this pull request as ready for review March 31, 2022 19:36
@albi3ro
Copy link
Contributor

albi3ro commented Apr 5, 2022

Was something about qml.device('ionq.qpu', target='qpu.harmony', wires=wires) not working?

Does this PR just make it slightly easier to create, fixing something that was broken, or add a new feature?

@Cynocracy
Copy link
Contributor Author

That method works fine for now, but we're working on new QPU backend targets, and thought it would map more cleanly into pennylane by adding support here.

Are there any reasons to prefer having support in Pennylane itself for different sized QPU backends?

If not, I can scrap this PR and keep with the target="" approach

@Cynocracy
Copy link
Contributor Author

Tested out that arg, it doesn't seem like we're able to send it through the qml device like you mentioned.

I would think that would work for us though, perhaps it's just some missing wiring?

Cynocracy added a commit to Cynocracy/PennyLane-IonQ that referenced this pull request Apr 5, 2022
Alternative to PennyLaneAI#49 for supporting more targets.
@Cynocracy
Copy link
Contributor Author

Cut #50 as an alternative to this approach

@josh146
Copy link
Member

josh146 commented Apr 7, 2022

@Cynocracy let us know when this PR (and also #50) are ready for review :)

@Cynocracy
Copy link
Contributor Author

Aye aye! I don't mind closing this in favor of #50 will comment over there when ready for review, am not sure why the tests are failing.

@Cynocracy Cynocracy closed this Apr 7, 2022
@Cynocracy Cynocracy deleted the backends branch April 7, 2022 20:10
antalszava pushed a commit that referenced this pull request Apr 13, 2022
* Allow overriding target on device

Alternative to #49 for supporting more targets.

* Fixup tests, add test case
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.

3 participants