Avoid private methods when adding unused Parameters (#34)

Using private methods of `QuantumCircuit` to force tracking of unused
`Parameter` instances resulting in the cleanest circuit output, but was
fragile against Qiskit changing the private data structure internals.
This has been a real problem as Qiskit moves more of the internal data
tracking down to Rust space.

This changes the hacked-in tracking to use only public methods to insert
a dummy reference, at the cost that a _true_ reference is added in to
the global phase.  For most real-world uses of unused parameters (i.e.
those in gate bodies), this will immediately be assigned out and so be
invisible to users.  The only place where this should appear to users is
if there is an `input float` that is unused.  In these cases, a dummy
reference will be inserted into the global phase that has no effect.

releasenotes/notes/parameter-hack-0ce14b3bf824981d.yaml

@@ -0,0 +1,14 @@
+---
+fixes:
+  - |
+    The internal-only method of handling circuits that define a parameter that they do not use
+    (such as a program that defines ``input float a;`` but doesn't use ``a``) has changed to
+    avoid using private Qiskit methods.  This makes it more resilient to changing versions
+    of Qiskit.
+upgrade:
+  - |
+    OpenQASM 3 inputs that include ``input float`` parameters that are not used by the program
+    will now parse to circuits that have a global phase that appears parametrised in terms of
+    the otherwise-unused parameter.  The numerical value of the global phase will not be affected,
+    and the global phase will be independent of the parameters.  You can discard the parameter
+    by using ``QuantumCircuit.assign_parameters`` to assign any numeric value to the parameter.

src/qiskit_qasm3_import/converter.py

@@ -33,7 +33,6 @@
     QuantumRegister,
     Qubit,
 )
-from qiskit.circuit.parametertable import ParameterReferences
 from qiskit.transpiler import Layout
 from qiskit.transpiler.layout import TranspileLayout
 from qiskit.circuit.library import standard_gates as _std
@@ -42,7 +41,7 @@
 from .data import Scope, Symbol
 from .exceptions import ConversionError, raise_from_node
 from .expression import ValueResolver, resolve_condition
-from .state import State, LocalScope, GateScope
+from .state import State, LocalScope, GateScope, add_dummy_parameter_reference
 
 _QASM2_IDENTIFIER = re.compile(r"[a-z]\w*", flags=re.ASCII)
 
@@ -153,6 +152,8 @@ def convert(self, node: ast.Program, *, source: Optional[str] = None) -> Quantum
                 input_qubit_mapping={bit: i for i, bit in enumerate(state.circuit.qubits)},
                 final_layout=None,
             )
+        for parameter in state.all_parameters - set(state.circuit.parameters):
+            add_dummy_parameter_reference(state.circuit, parameter)
         return state
 
     def _raise_previously_defined(self, new: Symbol, old: Symbol, node: ast.QASMNode) -> NoReturn:
@@ -218,16 +219,6 @@ def args():
 
         return zip(*args())
 
-    def _add_circuit_parameter(self, parameter: Parameter, context: State):
-        # TODO: this is a hack because Terra doesn't have any public way to add a parameter with
-        # no uses to a circuit, but we need to ensure that things work in later bindings if
-        # they're not all there.
-
-        # pylint: disable=protected-access
-        if parameter in context.circuit._parameter_table:
-            return
-        context.circuit._parameter_table[parameter] = ParameterReferences(())
-
     def _resolve_generic(self, node: ast.Expression, context: State) -> Tuple[Any, types.Type]:
         return ValueResolver(context).resolve(node)
 
@@ -324,7 +315,7 @@ def visit_QuantumGateDefinition(self, node: ast.QuantumGateDefinition, context:
                 inner.symbol_table.insert(
                     Symbol(parameter.name, parameter, types.Angle(), Scope.GATE, node)
                 )
-                self._add_circuit_parameter(parameter, inner)
+                inner.all_parameters.add(parameter)
             bits = [Qubit() for _ in node.qubits]
             inner.circuit.add_bits(bits)
             for name, bit in zip(node.qubits, bits):
@@ -446,7 +437,7 @@ def visit_IODeclaration(self, node: ast.IODeclaration, context: State) -> State:
         parameter = Parameter(name)
         symbol = Symbol(name, parameter, type, Scope.GLOBAL, node)
         context.symbol_table.insert(symbol)
-        self._add_circuit_parameter(parameter, context)
+        context.all_parameters.add(parameter)
         return context
 
     def visit_BreakStatement(self, node: ast.BreakStatement, context: State) -> State:
@@ -502,7 +493,7 @@ def visit_ForInLoop(self, node: ast.ForInLoop, context: State) -> State:
                 name = node.identifier.name
                 symbol = Symbol(name, parameter, var_type, Scope.LOCAL, node)
                 inner.symbol_table.insert(symbol)
-                self._add_circuit_parameter(parameter, inner)
+                inner.all_parameters.add(parameter)
                 for statement in node.block:
                     self.visit(statement, inner)
         return context

src/qiskit_qasm3_import/state.py

@@ -15,7 +15,7 @@
 import itertools
 import math
 
-from qiskit.circuit import QuantumCircuit
+from qiskit.circuit import QuantumCircuit, Parameter
 
 from qiskit.circuit.library import standard_gates as _std
 
@@ -52,6 +52,16 @@ def physical_qubit_index(name: Union[str, Symbol]) -> Optional[int]:
     return None
 
 
+def add_dummy_parameter_reference(circuit: QuantumCircuit, parameter: Parameter):
+    """Ensure that a circuit contains at least one reference to a given parameter."""
+    # TODO: this is a hack because Terra doesn't have any public way to add a parameter with
+    # no uses to a circuit, but we need to ensure that things work in later bindings if
+    # they're not all there.  This uses the fact that `parameter - parameter` is a
+    # `ParameterExpression` representation of zero that still tracks that it was once
+    # parametric over `parameter`.
+    circuit.global_phase += parameter - parameter
+
+
 class AddressingMode:
     """Addressing mode for qubits in OpenQASM 3 programs.
 
@@ -213,7 +223,15 @@ class State:
                   virtual, or hardware.
     """
 
-    __slots__ = ("scope", "_source", "circuit", "symbol_table", "_unique", "addressing_mode")
+    __slots__ = (
+        "scope",
+        "_source",
+        "circuit",
+        "symbol_table",
+        "_unique",
+        "addressing_mode",
+        "all_parameters",
+    )
 
     def __init__(self, source: Optional[str] = None):
         # We use the entire source, because at the moment, that's what all the error messages
@@ -222,6 +240,7 @@ def __init__(self, source: Optional[str] = None):
         self.scope = Scope.GLOBAL
         self.symbol_table = SymbolTables()
         self.addressing_mode = AddressingMode()
+        self.all_parameters = set()
         self.circuit = QuantumCircuit()
         self._unique = (f"_{x}" for x in itertools.count())
         self._finish_init()
@@ -237,6 +256,7 @@ def _new_scope(cls, scope, context):
         new_context.symbol_table = context.symbol_table
         new_context.symbol_table.push(SymbolTable(scope))
         new_context.addressing_mode = context.addressing_mode
+        new_context.all_parameters = set()
         new_context._source = context._source
         return new_context
 
@@ -270,6 +290,10 @@ def __enter__(self) -> State:
         return self._local_scope
 
     def __exit__(self, _exc_type, _exc_value, _traceback):
+        for parameter in self._local_scope.all_parameters - set(
+            self._local_scope.circuit.parameters
+        ):
+            add_dummy_parameter_reference(self._local_scope.circuit, parameter)
         self._local_scope.symbol_table.pop()
 
 
@@ -281,4 +305,6 @@ def __enter__(self) -> State:
         return self._gate_scope
 
     def __exit__(self, _exc_type, _exc_value, _traceback):
+        for parameter in self._gate_scope.all_parameters - set(self._gate_scope.circuit.parameters):
+            add_dummy_parameter_reference(self._gate_scope.circuit, parameter)
         self._gate_scope.symbol_table.pop()

tests/test_convert.py

@@ -340,6 +340,15 @@ def test_input_defines_parameter():
     assert qc == expected
 
 
+def test_unused_input_defines_parameter():
+    source = """
+        input float a;
+    """
+    qc = parse(source)
+    assert len(qc.parameters) == 1
+    assert qc.parameters[0].name == "a"
+
+
 def test_invalid_register_names_are_escaped():
     """Terra as of 0.22 only allows registers with valid OQ2 identifiers as names.  This restriction
     may be relaxed following Qiskit/qiskit-terra#9100."""
@@ -515,6 +524,22 @@ def test_parametrised_gate_definition():
     assert qc.data[0].operation.definition == expected
 
 
+def test_parametrised_gate_without_use():
+    # Test that a gate parametrised on an angle that's not actually used in the gate body works.
+    source = """
+        include 'stdgates.inc';
+        qubit q;
+        gate my_gate(p) a {
+            x a;
+        }
+        my_gate(0.5) q;
+    """
+    qc = parse(source)
+    assert len(qc.data) == 1
+    assert qc.data[0].operation.name == "my_gate"
+    assert qc.data[0].qubits == tuple(qc.qubits)
+
+
 def test_gate_cannot_redefine():
     source = """
         gate x q {