Update AQT Backend #6441

jbrixon · 2024-02-05T11:57:13Z

Adjusted the AQT backend so that it works with the new AQT Arnica API. We also updated the gateset to reflect what we support.
As we made an extension to the API, we were able to add a feature that lists the workspaces and resources that are available to a user. We also updated tests and documentation accordingly.

Fixes #6379

…irq into update-aqt

…-aqt

Update AQT Backend

google-cla · 2024-02-05T11:57:18Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

review-notebook-app · 2024-02-05T11:57:18Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

dstrain115 · 2024-02-09T00:30:24Z

cirq-aqt/cirq_aqt/aqt_device.py

        if len(operation.qubits) == 1:
            for idx in xtlk_arr.nonzero()[0]:
                exponent = operation.gate.exponent  # type:ignore
                exponent = exponent * xtlk_arr[idx]
-                xtlk_op = gate.on(system_qubits[idx]) ** exponent
+                xtlk_op = operation.gate.on(system_qubits[idx]) ** exponent


I think there needs to be a check here and on 138 to exclude the case when operation.gate is None.

Thanks! Fixed.

dstrain115 · 2024-02-09T00:35:14Z

cirq-aqt/cirq_aqt/aqt_sampler.py

@@ -40,16 +41,68 @@ class AQTSampler(cirq.Sampler):
    runs a single circuit or an entire sweep remotely
    """

-    def __init__(self, remote_host: str, access_token: str):
+    def __init__(self, workspace: str, resource: str, access_token: str, remote_host: str = "https://arnica.aqt.eu/api/v1/"):


Is there a reason to reorder the arguments? Is it because of the default argument?

If we keep the order the same, it would be more backwards compatible. This is doubly important since all the arguments are strings, so if people are passing them by position, they could get out of order.

Yes, it was because of the default argument. We preferred the definition of workspace and resource than requiring the user to piece together a URL, but this does require breaking backwards-compatibility. We figured that by requiring two arguments now, it would be clear enough that the "old way" is not the right way any more.

dstrain115 · 2024-02-09T00:35:49Z

cirq-aqt/cirq_aqt/aqt_sampler.py

@@ -40,16 +41,68 @@ class AQTSampler(cirq.Sampler):
    runs a single circuit or an entire sweep remotely
    """

-    def __init__(self, remote_host: str, access_token: str):
+    def __init__(self, workspace: str, resource: str, access_token: str, remote_host: str = "https://arnica.aqt.eu/api/v1/"):


Recommend putting "https://arnica.aqt.eu/api/v1/" into a constant at the top, such as
_DEFAULT_HOST = "https://arnica.aqt.eu/api/v1/"

Yes, agreed. Fixed.

dstrain115 · 2024-02-09T00:36:20Z

cirq-aqt/cirq_aqt/aqt_sampler.py

        self.remote_host = remote_host
        self.access_token = access_token

+    @staticmethod
+    def fetch_resources(access_token: str, remote_host: str = "https://arnica.aqt.eu/api/v1/") -> None:


Same here, put remote_host default as a constant.

dstrain115 · 2024-02-09T00:38:24Z

cirq-aqt/cirq_aqt/aqt_sampler.py

+            raise RuntimeError('Got unexpected return data from server: \n' + str(response.json()))
+
+        workspaces = cast(list, response.json())
+        col_widths = [19, 21, 20, 3]


What's with these random width numbers? It can't be 20, 20, 20?

These are completely arbitrary and just literally the current longest possible strings. Happy to change.

Maybe we should make it a constant. Or better yet, make it part of a different class rather than hardcode it in a sampler class. Also a line comment explaining that these are the current longest strings may be helpful.

There was no real reason to initialise the values like that, so have adjusted this slightly.

dstrain115 · 2024-02-09T00:40:39Z

cirq-aqt/cirq_aqt/aqt_sampler.py

+                col_widths[1] = max(col_widths[1], len(resource['name']))
+                col_widths[2] = max(col_widths[2], len(resource['id']))
+
+        print("+-" + col_widths[0]*"-"+ "-+-" + col_widths[1]*"-" + "-+-" + col_widths[2]*"-"  + "-+-" + col_widths[3]*"-" + "-+")


I would recommend creating a data structure here. Perhaps a dataclass or attrs that has workspace_id, resource_name, resource_id as member variables. Then, the function can return that. This will be more programmatically useful.

Optionally, you could then have print_resources somewhere that prints this out in this fixed width format.

We considered this, but we currently need the user to manually choose the workspace and resource they want to use. By allowing these to be chosen programmatically somehow, it could introduce errors that might lead to confusion. There is also limited information currently provided by the API with which one would programmatically choose - it is literally just a list of acceptable workspace/resource IDs.

This is intended purely as a helper function to allow users to look up the ID of resources that they already have access to, in case they forgot to write them down or something.

I still don't think this is good design. Having a Workspace class that contains names and ids of resources would be a natural way to encapsulate this information returned from the API, provide a good place to document what is returned, be more testable, and be more testable. You could then have a print() (or just put it in str) function on this class to print the information as it is here. It also would not take too much work to move this into a class.

I certainly agree it's not good design. Have adapted slightly to expose a fetch as well as a print static method.

Used TypedDict instead of classes to stay consistent with what was already there. Note sure in hindsight that that was the best idea.

dstrain115 · 2024-02-09T00:42:34Z

cirq-aqt/cirq_aqt/aqt_sampler.py

+    def _parse_legacy_circuit_json(self, json_str: str) -> list:
+        """Converts a legacy JSON circuit representation.
+
+        Converts a JSON created for the legacy API into one that will work


Do these APIs have versions? "Converts a JSON created for the API v0.3 ..." would be more precise and less prone to misinterpretation when the newer than new API comes out next year (or whenever)

Yes, now they do. Updated. Thanks!

dstrain115 · 2024-02-09T01:27:13Z

cirq-aqt/cirq_aqt/aqt_sampler.py

@@ -99,28 +152,78 @@ def _generate_json(
            raise RuntimeError('Cannot send an empty circuit')
        json_str = json.dumps(seq_list)
        return json_str
+
+    def _parse_legacy_circuit_json(self, json_str: str) -> list:


better typing would be "-> list[dict[str,str]]

It's not quite so simple, but I've improved this return type anyway with better definitions for the operations.

dstrain115 · 2024-02-09T01:28:49Z

cirq-aqt/cirq_aqt/aqt_sampler.py

@@ -223,10 +341,12 @@ class AQTSamplerLocalSimulator(AQTSampler):
    sampler.simulate_ideal=True
    """

-    def __init__(self, remote_host: str = '', access_token: str = '', simulate_ideal: bool = False):
+    def __init__(self, workspace: str = "", resource: str = "", access_token: str = "", remote_host: str = "", simulate_ideal: bool = False):


Same here, it would be nice to preserve backwards compatibility.

dstrain115 · 2024-02-09T01:31:14Z

Also, you can fix the formatting by running check/format --apply

dstrain115

LGTM with a few last nits. Thanks for addressing comments.

dstrain115 · 2024-03-20T14:08:20Z

cirq-aqt/cirq_aqt/aqt_device.py

@@ -36,11 +37,18 @@
 gate_dict = {'X': cirq.X, 'Y': cirq.Y, 'Z': cirq.Z, 'MS': cirq.XX, 'R': cirq.PhasedXPowGate}


+class OperationString(Enum):


We should add a short docstring for this enum.

Thanks - fixed.

dstrain115 · 2024-03-20T14:09:05Z

cirq-aqt/cirq_aqt/aqt_device.py

-        'Y': cirq.depolarize(1e-3),
-        'Z': cirq.depolarize(1e-3),
+        'R': cirq.depolarize(1e-3),
+        'Z': cirq.depolarize(0),


Should this use the enum values?

Yes! Fixed. Thanks

dstrain115 · 2024-03-29T14:48:13Z

It looks like there are some integration test failures. Once these are resolved, this should be good to merge!
Thanks for addressing all the requested changes!

dstrain115 · 2024-04-10T17:33:28Z

Sent you an invite to be a collaborator, which should allow you to run CI on your own.

jbrixon · 2024-04-15T13:11:53Z

Sent you an invite to be a collaborator, which should allow you to run CI on your own.

Thanks. Have a weird mismatch between formatting locally and on CI. Was missing some test coverage too, but let's pretend that didn't happen..! Looks like everything's passing now finally...

codecov · 2024-04-15T13:23:35Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.79%. Comparing base (21986dd) to head (dab7e05).

❗ Current head dab7e05 differs from pull request most recent head fd6e8c9. Consider uploading reports for the commit fd6e8c9 to get more accurate results

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #6441    +/-   ##
========================================
  Coverage   97.78%   97.79%            
========================================
  Files        1124     1124            
  Lines       95493    95680   +187     
========================================
+ Hits        93380    93570   +190     
+ Misses       2113     2110     -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Adjusted the AQT backend so that it works with the new AQT Arnica API. We also updated the gateset to reflect what we support. As we made an extension to the API, we were able to add a feature that lists the workspaces and resources that are available to a user. We also updated tests and documentation accordingly. Fixes quantumlib#6379

jbrixon and others added 15 commits January 22, 2024 15:39

Update supported gates, API interface

2a5f6f8

Update params for sampler local sim

e696864

Update docs

8b56a64

Remove unsupported match statement

c318179

updated access.md

783d624

Merge branch 'update-aqt' of github.com:alpine-quantum-technologies/C…

1892647

…irq into update-aqt

intermediate version of getting started

f762a64

Add static method to list workspaces and resources

1ca7f1e

Remove arbitrary host params

f9c0150

Update documentation

95d5368

Merge remote-tracking branch 'upstream/main' into update-aqt

73e3848

Update noise model

919c037

Remove unused cast

3a77f78

Merge branch 'main' of https://github.com/quantumlib/Cirq into update…

17ceb6a

…-aqt

Merge pull request #1 from alpine-quantum-technologies/update-aqt

86f5eab

Update AQT Backend

jbrixon requested review from ma5x, pschindler, vtomole, cduck and a team as code owners February 5, 2024 11:57

jbrixon requested a review from MichaelBroughton February 5, 2024 11:57

CirqBot added the size: L 250< lines changed <1000 label Feb 5, 2024

dstrain115 reviewed Feb 9, 2024

View reviewed changes

jbrixon added 4 commits February 12, 2024 08:31

Improve return type of new method

7f710ef

Move default host to constant

22b169d

Raise error if op gate is None

b6c4a8d

Fix get crosstalk operation

6eedf21

jbrixon added 5 commits March 1, 2024 15:33

Refactor fetch resource static function

ddfc44d

Add enum for operation strings

61c9205

Fix formatting

8617718

Fix type errors

cd39a7a

Linting

43972cb

dstrain115 approved these changes Mar 20, 2024

View reviewed changes

jbrixon added 4 commits March 26, 2024 11:27

Add enum docs

950e84a

Merge branch 'main' of https://github.com/quantumlib/Cirq

4ace5b9

Remove unsupported import

270b833

Fix noise dict keys

6ac86f4

jbrixon added 3 commits April 3, 2024 08:52

Format

c708567

Fix enum string conversion

0dac7f0

Merge branch 'main' of https://github.com/quantumlib/Cirq

0f99427

jbrixon and others added 7 commits April 15, 2024 12:36

Fix formatting

6186b62

Remove redundant counter

3fb723d

Add test for operation with None gate

8e2eba7

Add tests for static methods

234e38e

Remove unreachable code

e6a32a0

Fix formatting

4480c38

Merge branch 'main' into main

dab7e05

dstrain115 enabled auto-merge (squash) April 16, 2024 18:35

dstrain115 added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Apr 16, 2024

Merge branch 'main' into main

fd6e8c9

dstrain115 merged commit e1b03ef into quantumlib:main Apr 19, 2024
29 checks passed

fg-aqt mentioned this pull request May 7, 2024

Docs in quantumai.google not up to date #6587

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update AQT Backend #6441

Update AQT Backend #6441

jbrixon commented Feb 5, 2024

google-cla bot commented Feb 5, 2024

review-notebook-app bot commented Feb 5, 2024

dstrain115 Feb 9, 2024

jbrixon Feb 12, 2024

dstrain115 Feb 9, 2024

jbrixon Feb 12, 2024

dstrain115 Feb 9, 2024

jbrixon Feb 12, 2024

dstrain115 Feb 9, 2024

jbrixon Feb 12, 2024

dstrain115 Feb 9, 2024

jbrixon Feb 12, 2024

dstrain115 Feb 23, 2024

jbrixon Mar 1, 2024

dstrain115 Feb 9, 2024

jbrixon Feb 12, 2024

dstrain115 Feb 23, 2024

jbrixon Mar 1, 2024

dstrain115 Feb 9, 2024

jbrixon Feb 12, 2024

dstrain115 Feb 9, 2024

jbrixon Feb 12, 2024

dstrain115 Feb 9, 2024

dstrain115 commented Feb 9, 2024

dstrain115 left a comment

dstrain115 Mar 20, 2024

jbrixon Mar 26, 2024

dstrain115 Mar 20, 2024

jbrixon Mar 26, 2024

dstrain115 commented Mar 29, 2024 •

edited

Loading

dstrain115 commented Apr 10, 2024

jbrixon commented Apr 15, 2024

codecov bot commented Apr 15, 2024 •

edited

Loading

		@@ -36,11 +37,18 @@
		gate_dict = {'X': cirq.X, 'Y': cirq.Y, 'Z': cirq.Z, 'MS': cirq.XX, 'R': cirq.PhasedXPowGate}


		class OperationString(Enum):

Update AQT Backend #6441

Update AQT Backend #6441

Conversation

jbrixon commented Feb 5, 2024

google-cla bot commented Feb 5, 2024

review-notebook-app bot commented Feb 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dstrain115 commented Feb 9, 2024

dstrain115 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dstrain115 commented Mar 29, 2024 • edited Loading

dstrain115 commented Apr 10, 2024

jbrixon commented Apr 15, 2024

codecov bot commented Apr 15, 2024 • edited Loading

Codecov Report

dstrain115 commented Mar 29, 2024 •

edited

Loading

codecov bot commented Apr 15, 2024 •

edited

Loading