correlated loss support #526
Conversation
| traits = frozenset({lowering.FromPythonCall()}) | ||
| nonce: int = ( | ||
| info.attribute() | ||
| nonce: int = info.attribute( |
There was a problem hiding this comment.
I think the correlated error previously used to exist in the code base. I'm wondering if the previous approach also assigned a random number to nonce or if there is a better solution.
There was a problem hiding this comment.
The nonce solution was mine originally. If you can find a stim instruction other than I_ERROR that satisfies the following, then please use it.
- The instruction as emitted from kirin must have no effect when simulated in stim
- The instruction must maintain groups of qubits, either within a single instruction or by the stim library not merging multiple instructions in sequence.
There was a problem hiding this comment.
I agree that I_ERROR is the only choice.
I'm just wondering if a random 32 bit integer for the tag is the best solution or if, e.g., a global counter would be better.
There was a problem hiding this comment.
the tag need to be different in this case? how does it work
There was a problem hiding this comment.
@kaihsin When you import the stim IR into the stim package (stim.Circuit(...)) it optimizes the instructions by merging adjacent commands if they are the same instruction, have the same tag, and same argument values. They merge by concatenating the list of qubits targeted. E.g. CX 1 2 3 4; CX 2 4->CX 1 2 3 4 2 4.
There was a problem hiding this comment.
@rafaelha A counter would also work as long as you can ensure no collisions.
There was a problem hiding this comment.
Alternative solution would be to put dummy I statements between each correlated error to block merging.
E.g.
I_ERROR[correlated_error](0.01) 1 2 3 4 5
I
I_ERROR[correlated_error](0.01) 6 7 8
I
I_ERROR[correlated_error](0.01) 1 3 7
There was a problem hiding this comment.
I think I still prefer the random nonce. The I works, but seems a bit dangerous if some parser were to discard them.
There was a problem hiding this comment.
I_ERROR[correlated_error](0.01) 1 2 3 4 5
I_ERROR[correlated_error_barrier]
I_ERROR[correlated_error](0.01) 6 7 8
I_ERROR[correlated_error_barrier]
I_ERROR[correlated_error](0.01) 1 3 7
johnzl-777
changed the title
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
For python 3.10, cirq 1.5.0 is used since later versions only support 3.11. Here, the test suite raises 10k warning which all originate from within cirq.
|
Coverage is failing but that's due to the skipped files (due to ongoing refactor). A test in |
|
|
||
| [tool.pytest.ini_options] | ||
| testpaths = "test/" | ||
| filterwarnings = [ |
There was a problem hiding this comment.
The python 3.10 build is producing a lot of warnings (this currently also happens on main)
test/cirq_utils/test_parallelize.py: 9170 warnings
test/qasm2/test_native.py: 1873 warnings
/Users/rafaelhaenel/Documents/quera/kirin-workspace/.venv/lib/python3.10/site-packages/cirq/circuits/circuit_operation.py:173: FutureWarning: In cirq 1.6 the default value of `use_repetition_ids` will change to
`use_repetition_ids=False`. To make this warning go away, please pass
explicit `use_repetition_ids`, e.g., to preserve current behavior, use
CircuitOperations(..., use_repetition_ids=True)
warnings.warn(msg, FutureWarning)
These warnings are coming internally from cirq 1.5.0. The solution is to simply upgrade to cirq 1.6 which requires Python 3.11 (which is why only the 3.10 build has these issues).
I've gone ahead and filtered these warnings.
| p (float): Probability of the qubits being lost. | ||
| qubits (IList[Qubit, Any]): The list of qubits to which the correlated noise channel is applied. | ||
| """ | ||
| broadcast.correlated_qubit_loss(p, qubits) |
There was a problem hiding this comment.
This is somewhat hacky. correlated_qubit_loss is an n-qubit operator. As far as I know all other operators act on either one or two qubits.
Stim does not support n-qubit operators, which is why the nonce workaround is introduced.
I'm wondering if broadcast should be disallowed for this type of operator? Instead only use apply? I am a bit confused how that works after the refactor.
There was a problem hiding this comment.
This mismatch in semantics is a concern. There are a couple stim gates that act on more qubits: MPP, SPP, and SPP_DAG
# Perform a SQRT_YY gate between qubit 1 and 2, and a SQRT_ZZ_DAG between qubit 3 and 4.
SPP Y1*Y2 !Z1*Z2
There was a problem hiding this comment.
What are the requirements here? Do we really need to support correlated loss for more than two qubits at a time? If so, it's probably best to make this statement apply to an IList[IList[Qubit]] or similar. All statements in the gate dialect should "broadcast", meaning that it applies to each entry in the list of arguments.
@cduck are you saying we need to support statements for these gates? Or are you just pointing out that these would be points where this noise process can be injected and so we have more than two qubits here?
There was a problem hiding this comment.
From the example @cduck sketched out for me I think it should be an arbitrary number of atoms.
From Casey himself:
What I mean is
I_ERROR[CORRELATED_LOSS:<nonce>](p) 1 2 3
Is there is a chance p that a single event that looses all atoms 1, 2, and 3 occurs.
Briefly chatted with @david-pl , IList[IList[Qubit]] makes sense here.
In the apply case you just accept a single list of qubits (if one atom in the list goes, all the other ones get lost too)
In the broadcast case you accept list of lists, if any atom in one of those sublists goes, then the other atoms should go too
There was a problem hiding this comment.
yes in the case of SPP is why we have discussion whether we need PauliString Type or something to express a pauli-string. iirc the conclusion is to just use string "XYXXYZ" to represent SPP in stim dialect, and no support for it in squin?
There was a problem hiding this comment.
We don't need to have an equivalent of the SPP command in kirin-stim at all. This is a really niche gate that we don't need. I was just using that as an example of a more complex stim instruction than I_ERROR.
@cduck are you saying we need to support statements for these gates? Or are you just pointing out that these would be points where this noise process can be injected and so we have more than two qubits here?
@david-pl, I'm throwing out ideas for stim gates that can be repurposed to represent correlated atom loss. SPP isn't a great option but shows how stim can group qubits.
There was a problem hiding this comment.
@cduck ok so thats align with what we discuss earlier with @ChenZhao44 that we don't really have need of SPP. For the tag. If thats the behavior of stim, then, is the quest here that you want to also preserve the command order in stim circuit python instance (i.e. don't want stim to auto merge? so we need separate tags?)
… instead of list[Qubit]
…tangular qubit matrices
5 participants
This pull request adds support for correlated qubit loss noise channel to
squin.noisedialect and associated stdlibstimdialect