Closed saich08 closed 1 year ago
Thanks for your contribution. I have also received confirmation that you have signed the CLA. I will proceed to review this PR over the coming days.
One quick comment, you need to update the CI configuration so that the required dependencies for Azure Quantum are installed properly when setting up all the build environments.
There’s also an issue with the build requirements in pyproject.toml
with setuptools-scm
v7+ and Python 3.6.
I will address this in a separate PR so don’t worry too much about those CI failures for now.
NB: will need to wait for #440 to be merged to fix the CI failures for Python 3.6 and older.
I went through the changes in this PR and there are more than a couple of places that I would like to see some changes implemented.
That being said, the proposed changes are well thought through and I will be happy to merge those and add compatibility with Azure Quantum soon.
NB: I will do a second review pass once you have addressed the changes in this PR. Also, please run
pre-commit
on your code to fix the formatting and see some of the remaining issues. You might want to run it aspre-commit run --all-files --hook-stage manual
Thanks @Takishima for detailed review. You can expect some update in couple of days. I'll respond your comments once I made necessary changes.
@Takishima, I responded your first pass comments. Please review when you got time. Most of CI tasks are successful now, except on <3.6 (as expected).
I think most of the issues I noticed last time are resolved. I will take another look at the changes again this week and then we can probably merge this PR if all looks good.
Totals | |
---|---|
Change from base Build 2759348897: | 0.0% |
Covered Lines: | 7296 |
Relevant Lines: | 7296 |
@Takishima, Addressed rest of the active comments as well. Please review when you get a chance.
Looks like few PR check failing are failing (3 • Clang 3.5 • x64
, 3 • GCC 7 • x64
and coveralls
). I'll see what I can do.
With regards the failures 3 • Clang 3.5 • x64
and 3 • GCC 7 • x64
, it looks like issues related to updating Python packages so don't worry too much about those. I will look into those and see if I can find an easy fix.
The coveralls problems are mostly due to to lack of test coverage in the following files:
Name Stmts Miss Cover Missing
-------------------------------------------------------------------------------------------
...
projectq/backends/_azure/_azure_quantum.py 168 15 91% 105, 160, 191-197, 239, 267-268, 295, 307, 327, 338, 350
projectq/backends/_azure/_azure_quantum_client.py 15 2 87% 26-27
projectq/backends/_azure/_exceptions.py 2 0 100%
projectq/backends/_azure/_util.py 120 10 92% 105, 144, 181, 188, 236, 243, 288, 300, 308-311
...
-------------------------------------------------------------------------------------------
TOTAL 7352 34 99%
I have managed to fix the CI failure issues by making sure pip
and setuptools
are up-to-date. Now the only remaining issues are linked to test coverage.
Otherwise, I think this PR looks good (though I will go through the code once more, but I don't expect to have anything else to complain about ;-) )
@Takishima, we should be good with coveralls now. I got 100% coverage for azure-quantum modules.
projectq/backends/_azure/_azure_quantum.py 158 0 100% projectq/backends/_azure/_azure_quantum_client.py 15 0 100% projectq/backends/_azure/_exceptions.py 2 0 100% projectq/backends/_azure/_utils.py 112 0 100%
Thanks for working that out. I will take another look at this next week.
The current issues stems for the fact that azure-quantum
for Python 3.6 is version 0.18.X
and the following code does not work properly:
from azure.quantum.target import IonQ, Quantinuum, Target
from azure.quantum.target.target_factory import TargetFactory
NB: this fails with the following error message:
...
Traceback (most recent call last):
File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'azure.quantum.target'
Is there any way to make this work with Python 3.6 and azure-quantum <= 0.18
?
Actually I just saw that Python 3.6 has reached its EOL. I will merge another PR to remove 3.6 from the CI and then we can merge this.
Just one more question. Otherwise I am happy to merge this.
Closed PR by mistake, sorry about that :).
Thanks @Takishima for great review comments and contributions to this PR. This has been great learning experience. I am looking forward to contributing more. Hoping to see this change in ProjectQ release soon :).
Hi Team,
This is the PR to integrate Azure Quantum as backend to ProjectQ framework. As of now, Azure Quantum supports hardware and simulators from IonQ and Quantinuum targets.
Please review, welcome all feedback!
Thanks!