Add JSON serialisations #7396

WingCode · 2025-05-30T17:34:17Z

pavoljuhas · 2025-06-05T01:17:00Z

@WingCode - thank you for working on this. The failing CI checks should give some pointers on what is amiss. Feel free to ask here if you need clarification.

codecov · 2025-06-07T03:08:55Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.69%. Comparing base (9d511e9) to head (4bcbb8b).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #7396    +/-   ##
========================================
  Coverage   98.69%   98.69%            
========================================
  Files        1112     1114     +2     
  Lines       97937    98097   +160     
========================================
+ Hits        96659    96819   +160     
  Misses       1278     1278

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

WingCode · 2025-06-07T20:44:18Z

@pavoljuhas I have fixed the CI checks.

dstrain115 · 2025-06-10T19:16:03Z

cirq-core/cirq/contrib/noise_models/noise_models.py

@@ -128,7 +178,18 @@ def noisy_moment(self, moment: cirq.Moment, system_qubits: Sequence[cirq.Qid]):
            return output if self._prepend else output[::-1]
        return moment

+    def _json_dict_(self) -> dict[str, object]:
+        return {'readout_decay_gate': self.readout_decay_gate, 'prepend': self._prepend}


Can we instead serialize the decay probability instead of the gate, similar to above? The gate is somewhat of an internal detail.

dstrain115 · 2025-06-10T19:16:06Z

cirq-core/cirq/contrib/noise_models/noise_models.py

@@ -54,7 +65,18 @@ def noisy_moment(self, moment: cirq.Moment, system_qubits: Sequence[cirq.Qid]):
        ]
        return output[::-1] if self._prepend else output

+    def _json_dict_(self) -> dict[str, object]:
+        return {'qubit_noise_gate': self.qubit_noise_gate, 'prepend': self._prepend}


I think we should serialize the probability rather than the gate.
(Ok to add that as a member variable if it makes it easier.

Same with other gates below.

pavoljuhas

The code for serialization of contrib classes should be moved under cirq.contrib to limit any dependency of non-contrib cirq on cirq.contrib. Please see inline comments for other suggestions.

pavoljuhas · 2025-06-11T02:41:15Z

cirq-core/cirq/json_resolver_cache.py

+        # Avoid importing the optional contrib module at import time which would
+        # require optional dependencies. Instead, import noise models lazily
+        # when the resolver dictionary is created.


The classes below should be moved to contrib_class_resolver. They can use standard imports there without triggering the CI check that prohibits use of cirq.contrib from non-contrib cirq.

Also, the cirq.contrib module should register its resolver on import in a similar way as cirq_google below:

Cirq/cirq-google/cirq_google/__init__.py

Lines 114 to 118 in 6bb6661

# Register cirq_google's public classes for JSON serialization.

from cirq.protocols.json_serialization import _register_resolver

from cirq_google.json_resolver_cache import _class_resolver_dictionary

_register_resolver(_class_resolver_dictionary)

pavoljuhas · 2025-06-11T02:50:15Z

cirq-core/cirq/protocols/json_test_data/DampedReadoutNoiseModel.json

@@ -0,0 +1,8 @@
+{


For the contrib classes we should add cirq-core/cirq/contrib/json_test_data/ directory in a similar fashion as for cirq-google/cirq_google/json_test_data. The json and repr files for contrib classes should be located there.

Also the TESTED_MODULES dictionary below should be updated with 'cirq.contrib' entry so that the test code would look for files in .../contrib/json_test_data.
(the key can be entered as f'cirq{"."}contrib' to skip the check for cirq.contrib)

Cirq/cirq-core/cirq/protocols/json_serialization_test.py

Lines 50 to 58 in 6bb6661

TESTED_MODULES: dict[str, _ModuleDeprecation | None] = {

'cirq_aqt': None,

'cirq_ionq': None,

'cirq_google': None,

'cirq_pasqal': None,

'cirq_rigetti': None,

'cirq.protocols': None,

'non_existent_should_be_fine': None,

}

pavoljuhas · 2025-06-11T03:05:46Z

cirq-core/cirq/protocols/json_test_data/spec.py

        'Simulator',
        'StabilizerSampler',
        'DEFAULT_RESOLVERS',
+        'NoiseModelFromNoiseProperties',


The 'NoiseModelFromNoiseProperties' should be removed from the should_not_be_serialized list if this PR can support serialization of NoiseModelFromNoiseProperties. If that is too complicated, it is OK to limit the scope here to other NoiseModel classes only and leave NoiseModelFromNoiseProperties for later.

pavoljuhas · 2025-06-11T03:08:25Z

cirq-google/cirq_google/experimental/noise_models/noise_models.py

+            tuple(sorted((q, p) for q, p in (self.depol_probs or {}).items())),
+            tuple(sorted((q, p) for q, p in (self.bitflip_probs or {}).items())),
+            tuple(sorted((q, p) for q, p in (self.decay_probs or {}).items())),


Those dictionaries have simple float values. Would it work with just using them as are?

pavoljuhas · 2025-06-11T03:14:58Z

cirq-google/cirq_google/experimental/noise_models/noise_models.py

+        obj = cls.__new__(cls)
+        obj.depol_probs = dict(depol_probs)
+        obj.bitflip_probs = dict(bitflip_probs)
+        obj.decay_probs = dict(decay_probs)


Please replace cls.__new__ with standard object construction cls(...) here and in other files. If _json_dict_ above returns all __init__ arguments, this method should be unnecessary and removed.

pavoljuhas · 2025-06-11T03:18:16Z

cirq-google/cirq_google/experimental/noise_models/noise_models.py

+        return {
+            'depol_probs': tuple((q, p) for q, p in (self.depol_probs or {}).items()),
+            'bitflip_probs': tuple((q, p) for q, p in (self.bitflip_probs or {}).items()),
+            'decay_probs': tuple((q, p) for q, p in (self.decay_probs or {}).items()),


will it work with returning the dictionaries as they are?
Also, consider using obj_to_dict_helper here.

pavoljuhas · 2025-06-11T03:20:29Z

cirq-google/cirq_google/experimental/noise_models/noise_models_test.py

+def test_per_qubit_noise_model_repr_and_json():
+    q0 = cirq.GridQubit(0, 0)
+    model = cirq_google.experimental.noise_models.PerQubitDepolarizingWithDampedReadoutNoiseModel(
+        depol_probs={q0: 0.1}, bitflip_probs={q0: 0.2}, decay_probs={q0: 0.3}
+    )
+    cirq.testing.assert_equivalent_repr(model, setup_code='import cirq\nimport cirq_google')
+    restored = type(model)._from_json_dict_(**model._json_dict_())
+    assert restored == model


This test should be unnecessary after adding PerQubitDepolarizingWithDampedReadoutNoiseModel.json and PerQubitDepolarizingWithDampedReadoutNoiseModel.repr to cirq_google/json_test_data.

pavoljuhas · 2025-06-11T03:23:05Z

cirq-core/cirq/contrib/__init__.py

@@ -24,3 +24,4 @@
 from cirq.contrib.qcircuit import circuit_to_latex_using_qcircuit as circuit_to_latex_using_qcircuit
 from cirq.contrib import json  # noqa: F401
 from cirq.contrib.circuitdag import CircuitDag as CircuitDag, Unique as Unique
+from cirq.contrib import noise_models as noise_models


We should try to avoid adding new import here. If cirq.contrib registers its class resolver here, the import of noise_models can be deferred to the resolver execution.

pavoljuhas · 2025-06-11T03:26:56Z

cirq-core/cirq/devices/thermal_noise_model.py

+            "cirq.devices.ThermalNoiseModel("
+            f"qubits={set(self.rate_matrix_GHz.keys())!r}, "
+            f"gate_durations_ns={self.gate_durations_ns!r}, "
+            f"heat_rate_GHz=None, cool_rate_GHz=None, dephase_rate_GHz=None, "


This drops the attributes' values, please fix.

pavoljuhas · 2025-06-11T03:30:38Z

cirq-core/cirq/devices/thermal_noise_model_test.py

@@ -342,3 +342,19 @@ def test_noisy_moment_two_qubit():
            [9.87330937e-01, 0, 0, 9.95013725e-01],
        ],
    )
+
+
+def test_thermal_noise_model_repr_and_json():


This test (and other similar ones) should be replaced by adding ThermalNoiseModel.json and ThermalNoiseModel.repr files to json_test_data. Please check instructions in https://github.com/quantumlib/Cirq/blob/main/docs/dev/serialization.md.

mhucka · 2025-06-11T16:43:57Z

FYI that @pavoljuhas is out on jury duty (a mandatory civic obligation in the USA) starting today and possibly more days (depending on unpredictable factors). If he had commented on the PR or this issue before and hasn't responded, that's why.

pavoljuhas · 2025-06-16T18:18:53Z

Please consider setting up a local python environment for development per https://github.com/quantumlib/Cirq/blob/main/docs/dev/development.md#setting-up-an-environment and testing the code on your side first. A full test suite takes a while to run, but you can get fast results using either check/pytest-changed-files or, since you work on serialization, by running

check/pytest -n0 cirq-core/cirq/protocols/json_serialization_test.py

You can also consult the CI logs to find what tests are failing and specifically running those.

WingCode · 2025-06-16T18:32:01Z

Please consider setting up a local python environment for development per https://github.com/quantumlib/Cirq/blob/main/docs/dev/development.md#setting-up-an-environment and testing the code on your side first. A full test suite takes a while to run, but you can get fast results using either check/pytest-changed-files or, since you work on serialization, by running
check/pytest -n0 cirq-core/cirq/protocols/json_serialization_test.py
You can also consult the CI logs to find what tests are failing and specifically running those.

Thank you for the helpful tip, and apologies for the oversight. I just realized that I was mistakenly pushing fixes to the wrong branch, which ended up triggering the CI unnecessarily. I’ll make sure to be more careful going forward.

pavoljuhas

I have not got through all files, but I think we can address the new comments before next iteration.

cirq-core/cirq/contrib/json_test_data/__init__.py

pavoljuhas · 2025-06-17T02:08:47Z

cirq-core/cirq/contrib/json_test_data/spec.py

@@ -0,0 +1,35 @@
+# pylint: disable=wrong-or-nonexistent-copyright-notice


Please replace with the actual copyright statement as in __init__.py.

pavoljuhas · 2025-06-17T02:10:04Z

cirq-core/cirq/contrib/json_test_data/spec.py

+        "QuantumVolumeResult",
+        "SwapPermutationGate",
+        "BayesianNetworkGate",


The first 3 classes are serializable and should NOT be listed here.

Please add the corresponding .repr and .json files instead.

After that you can replace the dictionary filtering below with

resolver_cache=_class_resolver_dictionary(),

pavoljuhas · 2025-06-17T02:14:42Z

cirq-core/cirq/contrib/noise_models/noise_models.py

+    @classmethod
+    def _from_json_dict_(cls, depol_prob, prepend, **kwargs):
+        return cls(depol_prob, prepend=prepend)


When _json_dict_ provides all __init__ arguments, the _from_json_dict_ method is redundant. Please remove here and in other similar instances in this PR.

Suggested change

@classmethod

def _from_json_dict_(cls, depol_prob, prepend, **kwargs):

return cls(depol_prob, prepend=prepend)

pavoljuhas · 2025-06-17T02:20:16Z

cirq-core/cirq/devices/thermal_noise_model.py

        self.require_physical_tag: bool = require_physical_tag
        self.skip_measurements: bool = skip_measurements
        self._prepend = prepend

+    def _value_equality_values_(self):
+        gate_times = tuple(sorted((k, v) for k, v in self.gate_durations_ns.items()))


gate_durations_ns can be used as is, no need for conversion here.

pavoljuhas · 2025-06-17T02:35:21Z

cirq-core/cirq/devices/thermal_noise_model.py

+        gate_times = {cirq.json_cirq_type(k): v for k, v in self.gate_durations_ns.items()}
+        return {
+            'gate_durations_ns': tuple(gate_times.items()),
+            'rate_matrix_GHz': tuple((q, m.tolist()) for q, m in self.rate_matrix_GHz.items()),


rate_matrix_GHz has derived values which should not be stored. Please use it only to recover the qubits for class init, which do not seem to be stored anywhere else. Otherwise, just store here only the init arguments with a minimum conversions needed.

pavoljuhas · 2025-06-17T02:36:46Z

cirq-core/cirq/devices/thermal_noise_model.py

+    def _json_dict_(self) -> dict[str, object]:
+        gate_times = {cirq.json_cirq_type(k): v for k, v in self.gate_durations_ns.items()}
+        return {
+            'gate_durations_ns': tuple(gate_times.items()),


No need to convert to tuple, please store as a dictionary (which are native to json).

pavoljuhas · 2025-06-17T02:43:22Z

cirq-core/cirq/devices/thermal_noise_model.py

+        prepend,
+        **kwargs,
+    ):
+        obj = cls.__new__(cls)


Please replace with standard cls(args) call. Introduction of cls.__new__ should have a very strong justification, otherwise it is often a sign the code needs another look.

cirq-core/cirq/devices/thermal_noise_model_test.py

pavoljuhas · 2025-06-17T02:48:43Z

cirq-core/cirq/devices/thermal_noise_model_test.py

+    json_text = cirq.to_json(model)
+    assert cirq.read_json(json_text=json_text) == model


This duplicates implicit testing that happens with addition of ThermalNoiseModel.json and ThermalNoiseModel.repr files. Please delete here and also for other instances where .json and .repr files exist.

Co-authored-by: Pavol Juhas <[email protected]>

Add JSON serialisations

9a0f8f1

WingCode requested review from wcourtney, vtomole, verult and a team as code owners May 30, 2025 17:34

github-actions bot added the size: M 50< lines changed <250 label May 30, 2025

Add test JSON and fix tests

6c0d9bb

github-actions bot added size: L 250< lines changed <1000 and removed size: M 50< lines changed <250 labels Jun 1, 2025

WingCode and others added 2 commits June 2, 2025 01:15

Fix tests

51d9035

Merge branch 'main' into add-ser-noise-models

bcc575c

mhucka added the unitaryHACK label Jun 3, 2025

mhucka and others added 4 commits June 5, 2025 19:46

Merge branch 'main' into add-ser-noise-models

8da202f

Fix pylint

0643e61

Add tests

e5fa84b

Add tests

4e98a55

WingCode added 2 commits June 7, 2025 13:13

Fix tests

52f72cd

Fix lint

7d29cad

WingCode and others added 4 commits June 9, 2025 22:36

Fix black

edb0c6d

Merge branch 'main' into add-ser-noise-models

2e9bbd9

Merge branch 'main' into add-ser-noise-models

9b8b7fe

Fix lint

b867010

dstrain115 reviewed Jun 10, 2025

View reviewed changes

pavoljuhas requested changes Jun 11, 2025

View reviewed changes

Merge branch 'main' into add-ser-noise-models

0f06ac3

WingCode mentioned this pull request Jun 11, 2025

NoiseModels are not serializable #3846

Closed

WingCode and others added 5 commits June 14, 2025 02:45

Merge branch 'main' into add-ser-noise-models

555495e

Address review comments

a3b9ea4

Fix lint

e46ebfe

Fix lint

45942cd

Fix tests

6957dea

Fix tests

0f3aab9

WingCode added 4 commits June 16, 2025 21:01

Fix tests

c8d88a7

Fix tests

ec315f0

Fix tests

e03d267

Fix tests

c6d3059

WingCode requested a review from pavoljuhas June 16, 2025 21:08

Merge branch 'quantumlib:main' into add-ser-noise-models

9c02b87

mhucka assigned mhucka and WingCode and unassigned mhucka Jun 17, 2025

pavoljuhas requested changes Jun 17, 2025

View reviewed changes

WingCode and others added 2 commits June 17, 2025 10:37

Update cirq-core/cirq/contrib/json_test_data/__init__.py

2d2af5c

Co-authored-by: Pavol Juhas <[email protected]>

Update cirq-core/cirq/devices/thermal_noise_model_test.py

4bcbb8b

Co-authored-by: Pavol Juhas <[email protected]>

	# Register cirq_google's public classes for JSON serialization.
	from cirq.protocols.json_serialization import _register_resolver
	from cirq_google.json_resolver_cache import _class_resolver_dictionary

	_register_resolver(_class_resolver_dictionary)

	TESTED_MODULES: dict[str, _ModuleDeprecation \| None] = {
	'cirq_aqt': None,
	'cirq_ionq': None,
	'cirq_google': None,
	'cirq_pasqal': None,
	'cirq_rigetti': None,
	'cirq.protocols': None,
	'non_existent_should_be_fine': None,
	}

		@@ -0,0 +1,35 @@
		# pylint: disable=wrong-or-nonexistent-copyright-notice

	@classmethod
	def _from_json_dict_(cls, depol_prob, prepend, **kwargs):
	return cls(depol_prob, prepend=prepend)

		json_text = cirq.to_json(model)
		assert cirq.read_json(json_text=json_text) == model

Add JSON serialisations #7396

Are you sure you want to change the base?

Add JSON serialisations #7396

Conversation

WingCode commented May 30, 2025

Uh oh!

pavoljuhas commented Jun 5, 2025

Uh oh!

codecov bot commented Jun 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

WingCode commented Jun 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pavoljuhas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pavoljuhas Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhucka commented Jun 11, 2025

Uh oh!

pavoljuhas commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WingCode commented Jun 16, 2025

Uh oh!

pavoljuhas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pavoljuhas Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jun 7, 2025 •

edited

Loading

pavoljuhas Jun 11, 2025 •

edited

Loading

pavoljuhas commented Jun 16, 2025 •

edited

Loading

pavoljuhas Jun 17, 2025 •

edited

Loading