Improve deprecation of unit and duration (#13247)

* Improve deprecation of unit and duration

The unit and duration attributes of QuantumCircuit and Instruction were
introduced in #13224 however the way the PR went about this was a bit to
abbreviated and it had several downsides. The most noticable was that it
introduced a significant runtime regression as the rust internals were
accessing the deprecated getter. This is fixed so that the rust code
doesn't try to access any deprecated attributes. However, more
importantly there were several edge cases not accoutned for in the PR.
Mainly, DAGCircuit's matching attribute were not deprecated, but then
also specific use cases of the attributes were not addressed. The
primary driver of the per instruction duration and unit were the
timeline visualization which didn't have a method to function without
these data attributes. This commit addresses this by adding a new
target argument that is used to provide a backend's target which is
the source of truth for durations in a post unit/duration `Instruction`.
The other notable edge case is the `Delay` instruction which will need
to retain the duration and unit attributes as the fields are integral
for that instruction. The `Delay` is updated to handle them explicitly
outside of the `Instrution` data model to avoid complications around the
deprecation.

* Apply suggestions from code review

Co-authored-by: Elena Peña Tapia <[email protected]>

* Fix lint

* Mention the target in constructor deprecation message

---------

Co-authored-by: Elena Peña Tapia <[email protected]>

crates/circuit/src/circuit_instruction.rs

@@ -637,15 +637,23 @@ impl<'py> FromPyObject<'py> for OperationFromPython {
         let extract_extra = || -> PyResult<_> {
             let unit = {
                 // We accept Python-space `None` or `"dt"` as both meaning the default `"dt"`.
-                let raw_unit = ob.getattr(intern!(py, "unit"))?;
+                let raw_unit = ob.getattr(intern!(py, "_unit"))?;
                 (!(raw_unit.is_none()
                     || raw_unit.eq(ExtraInstructionAttributes::default_unit(py))?))
                 .then(|| raw_unit.extract::<String>())
                 .transpose()?
             };
+            // Delay uses the `duration` attr as an alias for param[0] which isn't deprecated
+            // while all other instructions have deprecated `duration` so we want to access
+            // the inner _duration to avoid the deprecation warning's run time overhead.
+            let duration = if ob.getattr(intern!(py, "name"))?.extract::<String>()? != "delay" {
+                ob.getattr(intern!(py, "_duration"))?.extract()?
+            } else {
+                ob.getattr(intern!(py, "duration"))?.extract()?
+            };
             Ok(ExtraInstructionAttributes::new(
                 ob.getattr(intern!(py, "label"))?.extract()?,
-                ob.getattr(intern!(py, "duration"))?.extract()?,
+                duration,
                 unit,
                 ob.getattr(intern!(py, "condition"))?.extract()?,
             ))

crates/circuit/src/dag_circuit.rs

@@ -35,9 +35,12 @@ use hashbrown::{HashMap, HashSet};
 use indexmap::IndexMap;
 use itertools::Itertools;
 
-use pyo3::exceptions::{PyIndexError, PyRuntimeError, PyTypeError, PyValueError};
+use pyo3::exceptions::{
+    PyDeprecationWarning, PyIndexError, PyRuntimeError, PyTypeError, PyValueError,
+};
 use pyo3::intern;
 use pyo3::prelude::*;
+
 use pyo3::types::{
     IntoPyDict, PyDict, PyInt, PyIterator, PyList, PySequence, PySet, PyString, PyTuple, PyType,
 };
@@ -257,10 +260,10 @@ pub struct DAGCircuit {
     /// Global phase.
     global_phase: Param,
     /// Duration.
-    #[pyo3(get, set)]
+    #[pyo3(set)]
     duration: Option<PyObject>,
     /// Unit of duration.
-    #[pyo3(get, set)]
+    #[pyo3(set)]
     unit: String,
 
     // Note: these are tracked separately from `qubits` and `clbits`
@@ -455,6 +458,45 @@ impl DAGCircuit {
         })
     }
 
+    /// The total duration of the circuit, set by a scheduling transpiler pass. Its unit is
+    /// specified by :attr:`.unit`
+    ///
+    /// DEPRECATED since Qiskit 1.3.0 and will be removed in Qiskit 2.0.0
+    #[getter]
+    fn get_duration(&self, py: Python) -> PyResult<Option<Py<PyAny>>> {
+        imports::WARNINGS_WARN.get_bound(py).call1((
+            intern!(
+                py,
+                concat!(
+                    "The property ``qiskit.dagcircuit.dagcircuit.DAGCircuit.duration`` is ",
+                    "deprecated as of qiskit 1.3.0. It will be removed in Qiskit 2.0.0.",
+                )
+            ),
+            py.get_type_bound::<PyDeprecationWarning>(),
+            2,
+        ))?;
+        Ok(self.duration.as_ref().map(|x| x.clone_ref(py)))
+    }
+
+    /// The unit that duration is specified in.
+    ///
+    /// DEPRECATED since Qiskit 1.3.0 and will be removed in Qiskit 2.0.0
+    #[getter]
+    fn get_unit(&self, py: Python) -> PyResult<String> {
+        imports::WARNINGS_WARN.get_bound(py).call1((
+            intern!(
+                py,
+                concat!(
+                    "The property ``qiskit.dagcircuit.dagcircuit.DAGCircuit.unit`` is ",
+                    "deprecated as of qiskit 1.3.0. It will be removed in Qiskit 2.0.0.",
+                )
+            ),
+            py.get_type_bound::<PyDeprecationWarning>(),
+            2,
+        ))?;
+        Ok(self.unit.clone())
+    }
+
     #[getter]
     fn input_map(&self, py: Python) -> PyResult<Py<PyDict>> {
         let out_dict = PyDict::new_bound(py);

crates/circuit/src/operations.rs

@@ -450,17 +450,29 @@ impl StandardGate {
             extra_attrs.condition(),
         );
         if label.is_some() || unit.is_some() || duration.is_some() || condition.is_some() {
-            let kwargs = [
-                ("label", label.to_object(py)),
-                ("unit", extra_attrs.py_unit(py).into_any()),
-                ("duration", duration.to_object(py)),
-            ]
-            .into_py_dict_bound(py);
+            let kwargs = [("label", label.to_object(py))].into_py_dict_bound(py);
             let mut out = gate_class.call_bound(py, args, Some(&kwargs))?;
+            let mut mutable = false;
             if let Some(condition) = condition {
-                out = out.call_method0(py, "to_mutable")?;
+                if !mutable {
+                    out = out.call_method0(py, "to_mutable")?;
+                    mutable = true;
+                }
                 out.setattr(py, "condition", condition)?;
             }
+            if let Some(duration) = duration {
+                if !mutable {
+                    out = out.call_method0(py, "to_mutable")?;
+                    mutable = true;
+                }
+                out.setattr(py, "_duration", duration)?;
+            }
+            if let Some(unit) = unit {
+                if !mutable {
+                    out = out.call_method0(py, "to_mutable")?;
+                }
+                out.setattr(py, "_unit", unit)?;
+            }
             Ok(out)
         } else {
             gate_class.call_bound(py, args, None)

qiskit/circuit/delay.py

@@ -33,8 +33,12 @@ def __init__(self, duration, unit="dt"):
         """
         if unit not in {"s", "ms", "us", "ns", "ps", "dt"}:
             raise CircuitError(f"Unknown unit {unit} is specified.")
-
-        super().__init__("delay", 1, 0, params=[duration], unit=unit)
+        # Double underscore to differentiate from the private attribute in
+        # `Instruction`. This can be changed to `_unit` in 2.0 after we
+        # remove `unit` and `duration` from the standard instruction model
+        # as it only will exist in `Delay` after that point.
+        self.__unit = unit
+        super().__init__("delay", 1, 0, params=[duration])
 
     broadcast_arguments = Gate.broadcast_arguments
 
@@ -45,6 +49,17 @@ def inverse(self, annotated: bool = False):
     def c_if(self, classical, val):
         raise CircuitError("Conditional Delay is not yet implemented.")
 
+    @property
+    def unit(self):
+
+        return self.__unit
+
+    @unit.setter
+    def unit(self, value):
+        if value not in {"s", "ms", "us", "ns", "ps", "dt"}:
+            raise CircuitError(f"Unknown unit {value} is specified.")
+        self.__unit = value
+
     @property
     def duration(self):
         """Get the duration of this delay."""
@@ -81,7 +96,7 @@ def validate_parameter(self, parameter):
                 raise CircuitError(
                     f"Duration for Delay instruction must be positive. Found {parameter}"
                 )
-            if self.unit == "dt":
+            if self.__unit == "dt":
                 parameter_int = int(parameter)
                 if parameter != parameter_int:
                     raise CircuitError("Integer duration is expected for 'dt' unit.")

qiskit/circuit/instruction.py

@@ -103,7 +103,23 @@ def __init__(self, name, num_qubits, num_clbits, params, duration=None, unit="dt
         # list of instructions (and their contexts) that this instruction is composed of
         # empty definition means opaque or fundamental instruction
         self._definition = None
+        if duration is not None:
+            warnings.warn(
+                "Setting a custom duration per instruction is deprecated as of Qiskit "
+                "1.3.0. It will be removed in Qiskit 2.0.0. An instruction's duration "
+                "is defined in a backend's Target object.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
         self._duration = duration
+        if unit is not None and unit != "dt":
+            warnings.warn(
+                "Setting a custom unit for duration per instruction is deprecated as of Qiskit "
+                "1.3.0. It will be removed in Qiskit 2.0.0. An instruction's duration "
+                "is defined in a backend's Target object which has a fixed unit in seconds.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
         self._unit = unit
 
         self.params = params  # must be at last (other properties may be required for validation)
@@ -347,9 +363,9 @@ def duration(self):
         return self._duration
 
     @duration.setter
-    def duration(self, duration):
+    def duration(self, value):
         """Set the duration."""
-        self._duration = duration
+        self._duration = value
 
     @property
     @deprecate_func(since="1.3.0", removal_timeline="in Qiskit 2.0.0", is_property=True)
@@ -358,9 +374,9 @@ def unit(self):
         return self._unit
 
     @unit.setter
-    def unit(self, unit):
+    def unit(self, value):
         """Set the time unit of duration."""
-        self._unit = unit
+        self._unit = value
 
     @deprecate_func(
         since="1.2",

qiskit/converters/circuit_to_dag.py

@@ -73,6 +73,6 @@ def circuit_to_dag(circuit, copy_operations=True, *, qubit_order=None, clbit_ord
 
     dagcircuit = core_circuit_to_dag(circuit, copy_operations, qubit_order, clbit_order)
 
-    dagcircuit.duration = circuit.duration
-    dagcircuit.unit = circuit.unit
+    dagcircuit.duration = circuit._duration
+    dagcircuit.unit = circuit._unit
     return dagcircuit

qiskit/converters/dag_to_circuit.py

@@ -74,6 +74,6 @@ def dag_to_circuit(dag, copy_operations=True):
 
     circuit._data = circuit_data
 
-    circuit.duration = dag.duration
-    circuit.unit = dag.unit
+    circuit._duration = dag.duration
+    circuit._unit = dag.unit
     return circuit

qiskit/visualization/timeline/core.py

@@ -58,6 +58,7 @@
 import numpy as np
 
 from qiskit import circuit
+from qiskit.transpiler.target import Target
 from qiskit.visualization.exceptions import VisualizationError
 from qiskit.visualization.timeline import drawings, types
 from qiskit.visualization.timeline.stylesheet import QiskitTimelineStyle
@@ -138,11 +139,13 @@ def add_data(self, data: drawings.ElementaryData):
         self._collections[data.data_key] = data
 
     # pylint: disable=cyclic-import
-    def load_program(self, program: circuit.QuantumCircuit):
+    def load_program(self, program: circuit.QuantumCircuit, target: Target | None = None):
         """Load quantum circuit and create drawing..
 
         Args:
             program: Scheduled circuit object to draw.
+            target: The target the circuit is scheduled for. This contains backend information
+                including the instruction durations used in scheduling.
 
         Raises:
            VisualizationError: When circuit is not scheduled.
@@ -180,10 +183,30 @@ def load_program(self, program: circuit.QuantumCircuit):
             for bit_pos, bit in enumerate(bits):
                 if not isinstance(instruction.operation, not_gate_like):
                     # Generate draw object for gates
+                    if target is not None:
+                        duration = None
+                        op_props = target.get(instruction.operation.name)
+                        if op_props is not None:
+                            inst_props = op_props.get(tuple(instruction.qubits))
+                            if inst_props is not None:
+                                duration = inst_props.getattr("duration")
+
+                        if duration is None:
+                            # Warn here because an incomplete target isn't obvious most of the time
+                            warnings.warn(
+                                "Target doesn't contain a duration for "
+                                f"{instruction.operation.name} on {bit_pos}, this will error in "
+                                "Qiskit 2.0.",
+                                DeprecationWarning,
+                                stacklevel=3,
+                            )
+                            duration = instruction.operation.duration
+                    else:
+                        duration = instruction.operation.duration
                     gate_source = types.ScheduledGate(
                         t0=t0,
                         operand=instruction.operation,
-                        duration=instruction.operation.duration,
+                        duration=duration,
                         bits=bits,
                         bit_position=bit_pos,
                     )

qiskit/visualization/timeline/interface.py

@@ -20,8 +20,10 @@
 """
 
 from typing import Optional, Dict, Any, List, Tuple
+import warnings
 
 from qiskit import circuit
+from qiskit.transpiler.target import Target
 from qiskit.exceptions import MissingOptionalLibraryError
 from qiskit.visualization.exceptions import VisualizationError
 from qiskit.visualization.timeline import types, core, stylesheet
@@ -43,6 +45,7 @@ def draw(
     plotter: Optional[str] = types.Plotter.MPL.value,
     axis: Optional[Any] = None,
     filename: Optional[str] = None,
+    target: Optional[Target] = None,
     *,
     show_idle: Optional[bool] = None,
     show_barriers: Optional[bool] = None,
@@ -81,6 +84,7 @@ def draw(
             the plotters uses given `axis` instead of internally initializing a figure object.
             This object format depends on the plotter. See plotters section for details.
         filename: If provided the output image is dumped into a file under the filename.
+        target: The target for the backend the timeline is being generated for.
         show_idle: DEPRECATED.
         show_barriers: DEPRECATED.
 
@@ -361,6 +365,14 @@ def draw(
     temp_style = stylesheet.QiskitTimelineStyle()
     temp_style.update(style or stylesheet.IQXStandard())
 
+    if target is None:
+        warnings.warn(
+            "Target is not specified. In Qiskit 2.0.0 this will be required to get the duration of "
+            "instructions.",
+            PendingDeprecationWarning,
+            stacklevel=2,
+        )
+
     # update control properties
     if idle_wires is not None:
         temp_style["formatter.control.show_idle"] = idle_wires

releasenotes/notes/deprecate-unit-duration-48b76c957bac5691.yaml

@@ -1,4 +1,12 @@
 ---
+features_visualization:
+  - |
+    The :func:`.timeline_drawer` visualization function has a new argument
+    ``target``, used to specify a :class:`.Target` object for the
+    visualization. By default the function used the
+    :attr:`.Instruction.duration` to get the duration of a given instruction,
+    but specifying the target will leverage the timing details inside the
+    target instead.
 deprecations_circuits:
   - |
     The :attr:`.QuantumCircuit.unit` and :attr:`.QuantumCircuit.duration`
@@ -10,16 +18,28 @@ deprecations_circuits:
     guess based on the longest path on the sum of the duration of
     :class:`.DAGCircuit` and wouldn't ever correctly account for control flow
     or conditionals in the circuit.
+  - |
+    The :attr:`.DAGCircuit.unit` and :attr:`.DAGCircuit.duration`
+    attributes have been deprecated and will be removed in Qiskit 2.0.0. These
+    attributes were used to track the estimated duration and unit of that
+    duration to execute on the circuit. However, the values of these attributes
+    were always limited, as they would only be properly populated if the
+    transpiler were run with the correct settings. The duration was also only a
+    guess based on the longest path on the sum of the duration of
+    :class:`.DAGCircuit` and wouldn't ever correctly account for control flow
+    or conditionals in the circuit.
   - |
     The :attr:`.Instruction.duration` and :attr:`.Instruction.unit` attributes
-    have been deprecated and will be removed in Qiskit 2.0.0. These attributes
-    were used to attach a custom execution duration and unit for that duration
-    to an individual instruction. However, the source of truth of the duration
-    of a gate is the :class:`.BackendV2` :class:`.Target` which contains
-    the duration for each instruction supported on the backend. The duration of
-    an instruction is not something that's typically user adjustable and is
-    an immutable property of the backend. If you were previously using this
-    capability to experiment with different durations for gates you can
-    mutate the :attr:`.InstructionProperties.duration` field in a given
-    :class:`.Target` to set a custom duration for an instruction on a backend
-    (the unit is always in seconds in the :class:`.Target`).
+    have been deprecated and will be removed in Qiskit 2.0.0. This includes
+    setting the ``unit`` or ``duration`` arguments for any :class:`qiskit.circuit.Instruction`
+    or subclass. These attributes were used to attach a custom execution
+    duration and unit for that duration to an individual instruction. However,
+    the source of truth of the duration of a gate is the :class:`.BackendV2`
+    :class:`.Target` which contains the duration for each instruction supported
+    on the backend. The duration of an instruction is not something that's
+    typically user adjustable and is an immutable property of the backend. If
+    you were previously using this capability to experiment with different
+    durations for gates you can mutate the
+    :attr:`.InstructionProperties.duration` field in a given :class:`.Target` to
+    set a custom duration for an instruction on a backend (the unit is always in
+    seconds in the :class:`.Target`).