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.

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

Avoid private methods when adding unused Parameters #34

Merged
merged 1 commit into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions releasenotes/notes/parameter-hack-0ce14b3bf824981d.yaml
Original file line number Diff line number Diff line change
@@ -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.
21 changes: 6 additions & 15 deletions src/qiskit_qasm3_import/converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
30 changes: 28 additions & 2 deletions src/qiskit_qasm3_import/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

I guess this potentially does change the type of global phase from a float to parameter expression in Qiskit which is a bit unfortunate. But it should be normalized away if somebody does anything that uses the circuit for anything and global phase gets cast as a float.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it does, and what's worse, it makes the global phase compare unequal to 0 (because of the parametrisation). But it should only actually appear for OQ3 programs like

input float a;

where a isn't used, and I think storing the dummy reference in the global phase is probably less disruptive than adding a dummy gate to hold it.



class AddressingMode:
"""Addressing mode for qubits in OpenQASM 3 programs.

Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand All @@ -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

Expand Down Expand Up @@ -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()


Expand All @@ -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()
25 changes: 25 additions & 0 deletions tests/test_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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 {
Expand Down
Loading