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.

Sign up for GitHub

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

Jump to bottom

Thermal noise model #4673

Merged
merged 23 commits into from
Jan 4, 2022
Merged

Thermal noise model #4673

merged 23 commits into from
Jan 4, 2022

Conversation

95-martin-orion
Copy link
Collaborator

This PR is part of #4640. It adds the ThermalNoiseModel, which models thermalization of a qubit - i.e., transitions from the |1) state to the |0) state or vice-versa, or dephasing events.

#4672 is a prerequisite for this PR. The only files that need to be reviewed in this PR are:

  • cirq-core/cirq/devices/...
    • __init__.py
    • thermal_noise_model[_test].py

@95-martin-orion 95-martin-orion requested review from cduck, vtomole and a team as code owners November 12, 2021 18:19
@google-cla google-cla bot added the cla: yes Makes googlebot stop complaining. label Nov 12, 2021
@CirqBot CirqBot added the size: XL lines changed >1000 label Nov 12, 2021
@95-martin-orion 95-martin-orion mentioned this pull request Nov 12, 2021
Closed
dkafri
dkafri reviewed Nov 13, 2021
View reviewed changes
Copy link
Collaborator

@dkafri dkafri left a comment

Choose a reason for hiding this comment

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

I haven't had a chance to review the thermal_noise_model yet.

cirq-core/cirq/devices/noise_utils.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/noise_utils.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/noise_utils.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/noise_utils.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/noise_utils.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/noise_utils.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/noise_utils.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/insertion_noise_model.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/insertion_noise_model.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/insertion_noise_model.py Outdated Show resolved Hide resolved
95-martin-orion
95-martin-orion commented Nov 15, 2021
View reviewed changes
Copy link
Collaborator Author

@95-martin-orion 95-martin-orion left a comment

Choose a reason for hiding this comment

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

Replied to comments. Due to the PR structure, changes will be applied in other PRs and bubble up to this one.

A top-level note on OpIdentifier: I discussed this with @tanujkhattar in #4672, and we concluded that cirq.Gateset and cirq.GateFamily were the appropriate structures to use for identifying operations by gate type. Using those types will remove the need for OpIdentifier, as they can be compared directly with operations.

cirq-core/cirq/devices/insertion_noise_model.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/noise_utils.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/noise_utils.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/noise_utils.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/insertion_noise_model.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/noise_utils.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/noise_utils.py Outdated Show resolved Hide resolved
@95-martin-orion
Copy link
Collaborator Author

Rebased and force-pushed to capture changes in the earlier PRs in this sequence.

@MichaelBroughton MichaelBroughton self-assigned this Dec 15, 2021
MichaelBroughton
MichaelBroughton requested changes Dec 15, 2021
View reviewed changes
Copy link
Collaborator

@MichaelBroughton MichaelBroughton left a comment

Choose a reason for hiding this comment

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

Looking good. A few tweaks and cleanups and I think we should be ready to merge.

cirq-core/cirq/devices/__init__.py Show resolved Hide resolved
cirq-core/cirq/devices/insertion_noise_model.py Show resolved Hide resolved
cirq-core/cirq/devices/thermal_noise_model.py Show resolved Hide resolved
cirq-core/cirq/devices/thermal_noise_model.py Show resolved Hide resolved
cirq-core/cirq/devices/thermal_noise_model.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/thermal_noise_model.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/thermal_noise_model.py Show resolved Hide resolved
cirq-core/cirq/devices/thermal_noise_model.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/thermal_noise_model.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/thermal_noise_model.py Outdated Show resolved Hide resolved
@95-martin-orion
Copy link
Collaborator Author

Requesting input on @MichaelBroughton's comments from @dkafri, who wrote the internal version of this code. I could maybe stumble through the linear algebra here, but in particular I'd like to make sure we don't go against any decisions made in the original design of this model.

dkafri
dkafri reviewed Dec 16, 2021
View reviewed changes
Copy link
Collaborator

@dkafri dkafri left a comment

Choose a reason for hiding this comment

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

Some comments

cirq-core/cirq/devices/insertion_noise_model.py Show resolved Hide resolved
cirq-core/cirq/devices/thermal_noise_model.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/thermal_noise_model.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/thermal_noise_model.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/thermal_noise_model.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/thermal_noise_model.py Outdated Show resolved Hide resolved
cirq-core/cirq/devices/thermal_noise_model.py Outdated Show resolved Hide resolved
@95-martin-orion 95-martin-orion mentioned this pull request Dec 20, 2021
Closed
@95-martin-orion
Copy link
Collaborator Author

Comments have been addressed and this PR is ready for re-review.

@MichaelBroughton MichaelBroughton added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jan 4, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jan 4, 2022
@CirqBot
Copy link
Collaborator

CirqBot commented Jan 4, 2022

Automerge cancelled: A status check is failing.

@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Jan 4, 2022
@95-martin-orion
Copy link
Collaborator Author

cirq-google/cirq_google/engine/engine_validator_test.py::test_create_gate_set_validator has a flake where the generated circuit is too long; testing shows this flake occurs ~0.4% of the time.

Opened #4796 to track this; re-running checks for now.

@95-martin-orion 95-martin-orion merged commit 1812fdd into quantumlib:master Jan 4, 2022
MichaelBroughton pushed a commit to MichaelBroughton/Cirq that referenced this pull request Jan 22, 2022
@95-martin-orion @CirqBot @MichaelBroughton
Thermal noise model (quantumlib#4673)
a1e7d13 
* Remove files for subsequent PRs

* Add coverage for OpId

* Clarify if-case

* snake_case_gamma

* Add insertion noise model.

* Update to use new match methods

* Apply review cleanup

* Introduce Thermal noise model.

* Align with checks

* remove ThermalChannel ref

* Answer review comments

* Document mutli-type behavior.

* qubit_dims -> qubits

* Extract methods, clean docstrings

* Cache kraus_ops_from_rates

* Document edge behaviors.

* Handle other qubits nicely

Co-authored-by: Cirq Bot <[email protected]>
@95-martin-orion 95-martin-orion mentioned this pull request Jun 27, 2022
Closed
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
@95-martin-orion @CirqBot @rht
Thermal noise model (quantumlib#4673)
8e0ee6d 
* Remove files for subsequent PRs

* Add coverage for OpId

* Clarify if-case

* snake_case_gamma

* Add insertion noise model.

* Update to use new match methods

* Apply review cleanup

* Introduce Thermal noise model.

* Align with checks

* remove ThermalChannel ref

* Answer review comments

* Document mutli-type behavior.

* qubit_dims -> qubits

* Extract methods, clean docstrings

* Cache kraus_ops_from_rates

* Document edge behaviors.

* Handle other qubits nicely

Co-authored-by: Cirq Bot <[email protected]>
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
@95-martin-orion @CirqBot
Thermal noise model (quantumlib#4673)
d6311f5 
* Remove files for subsequent PRs

* Add coverage for OpId

* Clarify if-case

* snake_case_gamma

* Add insertion noise model.

* Update to use new match methods

* Apply review cleanup

* Introduce Thermal noise model.

* Align with checks

* remove ThermalChannel ref

* Answer review comments

* Document mutli-type behavior.

* qubit_dims -> qubits

* Extract methods, clean docstrings

* Cache kraus_ops_from_rates

* Document edge behaviors.

* Handle other qubits nicely

Co-authored-by: Cirq Bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelBroughton MichaelBroughton MichaelBroughton approved these changes

@cduck cduck Awaiting requested review from cduck

@vtomole vtomole Awaiting requested review from vtomole

@dkafri dkafri Awaiting requested review from dkafri

Assignees

@MichaelBroughton MichaelBroughton

Labels
cla: yes Makes googlebot stop complaining. size: XL lines changed >1000
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@95-martin-orion @CirqBot @MichaelBroughton @dkafri