C2QA / bosonic-qiskit

Extension of Qiskit to support hybrid boson-qubit simulations for the NQI C2QA effort.
BSD 2-Clause "Simplified" License
46 stars 11 forks source link

Kraus.py warnings for PhotonLossNoisePass and circuit gate naming #108

Closed liu-zixiong closed 8 months ago

liu-zixiong commented 9 months ago

Issue Currently, PhotonLossNoisePass._photon_loss_error() has two warning flags —

  1. if op.duration is None, return None
  2. if op.name.startswith("delay"), return None

The first flag occurs whenever I run a circuit with just a single cv_delay, but otherwise does not affect the output . The second flag prevents me from applying PhotonLossNoisePass to circuits with a single cv_delay gate.

import c2qa
from qiskit import ClassicalRegister, visualization

# Create circuit
qmr = c2qa.QumodeRegister(1, 1)
cr = ClassicalRegister(1)
circ = c2qa.CVCircuit(qmr, cr)

# Initialize qumode in Fock state |1>
circ.cv_initialize(1, qmr[0])

# Circuit is a single delay gate of 1 ms followed by measurement
circ.cv_delay(duration=1, qumode=qmr[0], unit="ms")
circ.cv_measure(qmr[0], cr)

# Simulate with noise pass
noise_pass = c2qa.kraus.PhotonLossNoisePass(photon_loss_rates=0.5, circuit=circ, time_unit="ms")
_, _, counts = c2qa.util.simulate(circ, noise_passes=noise_pass)

print(counts)
------------------------------------------------------------------------------------
> {'1': 1024}
> UserWarning: PhotonLossNoisePass ignores instructions without duration, you may need to schedule circuit in advance.
> UserWarning: Qiskit applies delays as single qubit gates. Qumode PhotonLossNoisePass will not be applied

Noise is not applied even though I am expecting amplitude damping.

Diagnostics Flag 1: The first warning occurs because _photon_loss_error is executed multiple times depending on the circuit. When I add print statements for diagnostics, the following outputs appear when I run the above circuit again:

# Edited _photon_loss_error
def _photon_loss_error(self, op: Instruction, qubits: Sequence[int]):
        """Return photon loss error on each operand qubit"""
        error = None

        if self.applies_to_instruction(op, qubits):
            print(f"1. op.duration state is {op.duration}") ## <-
            if not op.duration:
                print(f"2. not op.duration state is {not op.duration}") ## <-
                if op.duration is None:
                    warnings.warn(
                        "PhotonLossNoisePass ignores instructions without duration,"
                        " you may need to schedule circuit in advance.",
                        UserWarning,
                    )
                return None
...

# If I run the above circuit again, the following output is generated:
------------------------------------------------------------------------------------
>1. op.duration state is None
>2. not op.duration state is True
>1. op.duration state is 1
>1. op.duration state is None
>2. not op.duration state is True
>1. op.duration state is None
>2. not op.duration state is True

It seems that because _photon_loss_error() is called multiple times, the UserWarning will be generated even if a duration is provided. The code should probably be changed so that when the duration is provided, no warning is generated.

Flag 2: The second warning happens because of how the label for cv_delay gate is label="delay(" + str(duration) + " " + unit +")". This means that there is currently no difference between how cv_delay and qiskit's delay gates are labelled, and so _photon_loss_error() is unable to differentiate between the two cases when they're called.

This issue has an easy fix — replacing the label for cv_delay to label="cv_delay(" + str(duration) + " " + unit +")" would be sufficient. But to prevent similar issues from happening with other gates, maybe it would be good practice to include a 'cv_' string in front of all bosonic qiskit gates? Doing so would however require a comprehensive renaming of all of the existing gates in circuit.py.

kevincsmith commented 8 months ago

@tjstavenger-pnnl I cannot see any reason why the proposed rename would cause issues. Okay if we go ahead and adopt this change for cv_delay? @liu-zixiong has the fix ready to be pushed into main.

tjstavenger-pnnl commented 8 months ago

Fine with me. Looks like you added cv_delay back in May, @kevincsmith.

liu-zixiong commented 8 months ago

Pushed cv_delay gate label rename to main. No other gates were renamed.