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

Add ParameterExpression.numeric to cast to number #11109

Merged
merged 2 commits into from
Dec 19, 2023
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
46 changes: 46 additions & 0 deletions qiskit/circuit/parameterexpression.py
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,52 @@ def is_real(self):
return False
return symbol_expr.is_real

def numeric(self) -> int | float | complex:
"""Return a Python number representing this object, using the most restrictive of
:class:`int`, :class:`float` and :class:`complex` that is valid for this object.

In general, an :class:`int` is only returned if the expression only involved symbolic
integers. If floating-point values were used during the evaluation, the return value will
be a :class:`float` regardless of whether the represented value is an integer. This is
because floating-point values "infect" symbolic computations by their inexact nature, and
symbolic libraries will use inexact floating-point semantics not exact real-number semantics
when they are involved. If you want to assert that all floating-point calculations *were*
carried out at infinite precision (i.e. :class:`float` could represent every intermediate
value exactly), you can use :meth:`float.is_integer` to check if the return float represents
an integer and cast it using :class:`int` if so. This would be an unusual pattern;
typically one requires this by only ever using explicitly :class:`~numbers.Rational` objects
while working with symbolic expressions.

This is more reliable and performant than using :meth:`is_real` followed by calling
:class:`float` or :class:`complex`, as in some cases :meth:`is_real` needs to force a
floating-point evaluation to determine an accurate result to work around bugs in the
upstream symbolic libraries.

Returns:
A Python number representing the object.

Raises:
TypeError: if there are unbound parameters.
"""
if self._parameter_symbols:
raise TypeError(
f"Expression with unbound parameters '{self.parameters}' is not numeric"
)
if self._symbol_expr.is_integer:
# Integer evaluation is reliable, as far as we know.
return int(self._symbol_expr)
# We've come across several ways in which symengine's general-purpose evaluators
# introduce spurious imaginary components can get involved in the output. The most
# reliable strategy "try it and see" while forcing real floating-point evaluation.
try:
real_expr = self._symbol_expr.evalf(real=True)
except RuntimeError:
# Symengine returns `complex` if any imaginary floating-point enters at all, even if
# the result is zero. The best we can do is detect that and decay to a float.
out = complex(self._symbol_expr)
return out.real if out.imag == 0.0 else out
Copy link
Member

Choose a reason for hiding this comment

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

should this be a math.isclose? Zero-checking floats is always hard to me...

Copy link
Member Author

Choose a reason for hiding this comment

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

Personally I'd say no - the only times we've seen uses where bad imaginary components enter is from bugs within Symengine itself (which this function works around), where the coefficient scales with the operation being done, so could be arbitrarily large. Auto clipping to zero also makes it tricky for people who were expecting to deal with very small numbers (which floating point is good at).

As an interface thing, a user can always clip the output themselves if needed, so I think not clipping is the safer choice.

return float(real_expr)

def sympify(self):
"""Return symbolic expression as a raw Sympy or Symengine object.

Expand Down
16 changes: 4 additions & 12 deletions qiskit/circuit/quantumcircuit.py
Original file line number Diff line number Diff line change
Expand Up @@ -3081,15 +3081,7 @@ def assign_parameters( # pylint: disable=missing-raises-doc
target._parameter_table[parameter] = ParameterReferences(())
target._parameter_table[parameter].add((operation, index))
if not new_parameter.parameters:
if new_parameter.is_real():
new_parameter = (
int(new_parameter)
if new_parameter._symbol_expr.is_integer
else float(new_parameter)
)
else:
new_parameter = complex(new_parameter)
new_parameter = operation.validate_parameter(new_parameter)
new_parameter = operation.validate_parameter(new_parameter.numeric())
elif isinstance(assignee, QuantumCircuit):
new_parameter = assignee.assign_parameters(
{to_bind: bound_value}, inplace=False, flat_input=True
Expand Down Expand Up @@ -3130,9 +3122,9 @@ def map_calibration(qubits, parameters, schedule):
for to_bind in contained:
parameter = parameter.assign(to_bind, parameter_binds.mapping[to_bind])
if not parameter.parameters:
parameter = (
int(parameter) if parameter._symbol_expr.is_integer else float(parameter)
)
parameter = parameter.numeric()
if isinstance(parameter, complex):
raise TypeError(f"Calibration cannot use complex number: '{parameter}'")
new_parameters[i] = parameter
modified = True
if modified:
Expand Down
33 changes: 4 additions & 29 deletions qiskit/transpiler/passes/basis/basis_translator.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,16 +319,8 @@ def _replace_node(self, dag, node, instr_map):
new_value = new_value.assign(*x)
else:
new_value = param.bind(bind_dict)
# cast from ParameterExpression to number, if no parameters left
if not new_value.parameters:
if new_value.is_real():
new_value = (
int(new_value)
if new_value._symbol_expr.is_integer
else float(new_value)
)
else:
new_value = complex(new_value)
new_value = new_value.numeric()
new_params.append(new_value)
new_op.params = new_params
else:
Expand All @@ -345,26 +337,9 @@ def _replace_node(self, dag, node, instr_map):
else:
new_phase = old_phase.bind(bind_dict)
if not new_phase.parameters:
if new_phase.is_real():
new_phase = (
int(new_phase)
if new_phase._symbol_expr.is_integer
else float(new_phase)
)
else:
# If is_real() evals false try casting to a float
# anyway in case there is a rounding error adding
# a near 0 complex term
try:
new_phase = float(new_phase)
except TypeError as exc:
raise TranspilerError(
f"Global phase: {new_phase} is complex which is invalid"
) from exc
try:
new_phase = float(new_phase)
except TypeError:
pass
new_phase = new_phase.numeric()
if isinstance(new_phase, complex):
raise TranspilerError(f"Global phase must be real, but got '{new_phase}'")
bound_target_dag.global_phase = new_phase
else:
bound_target_dag = target_dag
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
features:
- |
A new method :meth:`.ParameterExpression.numeric` is added, which converts a fully bound
parameter expression into the most restrictive builtin Python numeric type that accurately
describes the result of the symbolic evaluation. For example, a symbolic integer will become an
:class:`int`, while a symbolic real number will become a :class:`float` and a complex number
will become a :class:`complex`. This method includes several workarounds for peculiarities of
the evaluation contexts of ``symengine``, which can sometimes lead to spurious results when
calling :class:`complex` or :class:`float` on an expression directly.
26 changes: 26 additions & 0 deletions test/python/circuit/test_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -2005,6 +2005,32 @@ def test_bound_expression_is_real(self):
bound = x.bind({x: 1 + 1j})
self.assertEqual(bound.is_real(), False)

def test_numeric(self):
"""Tests of the 'numeric' method."""
a, b = Parameter("a"), Parameter("b")
one_int = (1 + a).assign(a, 0)
self.assertIsInstance(one_int.numeric(), int)
self.assertEqual(one_int.numeric(), 1)
one_float = (1.0 + a).assign(a, 0.0)
self.assertIsInstance(one_float.numeric(), float)
self.assertEqual(one_float.numeric(), 1.0)
one_imaginary = (1j + a).assign(a, 0.0)
self.assertIsInstance(one_imaginary.numeric(), complex)
self.assertEqual(one_imaginary.numeric(), 1j)

# This is one particular case where symengine 0.9.2 (and probably others) struggles when
Copy link
Member

Choose a reason for hiding this comment

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

This corner case can be removed with #11315 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd leave it in regardless - it still works as a regression test for us.

# evaluating in the complex domain, but gets the right answer if forced to the real domain.
# It appears more commonly because `symengine.Basic.subs` does not simplify the expression
# tree eagerly, so the `_symbol_expr` is `0.5 * (0.5)**2`. Older symengines then introduce
# a spurious small imaginary component when evaluating this `Mul(x, Pow(y, z))` pattern in
# the complex domain.
problem = (0.5 * a * b).assign(b, 0.5).assign(a, 0.5)
self.assertIsInstance(problem.numeric(), float)
self.assertEqual(problem.numeric(), 0.125)

with self.assertRaisesRegex(TypeError, "unbound parameters"):
(a + b).numeric()


class TestParameterEquality(QiskitTestCase):
"""Test equality of Parameters and ParameterExpressions."""
Expand Down