fix: validate FreeParameter name (#895)

* validate numerical free parameters

* simplify validation

* do not use _validate_name

---------

Co-authored-by: Viraj Chaudhari <72896239+virajvchaudhari@users.noreply.github.com>
Co-authored-by: Cody Wang <speller26@gmail.com>

src/braket/circuits/circuit.py

@@ -19,6 +19,7 @@
 
 import numpy as np
 import oqpy
+from sympy import Expr
 
 from braket.circuits import compiler_directives
 from braket.circuits.ascii_circuit_diagram import AsciiCircuitDiagram
@@ -472,7 +473,9 @@ def add_instruction(
 
         if self._check_for_params(instruction):
             for param in instruction.operator.parameters:
-                if isinstance(param, FreeParameterExpression):
+                if isinstance(param, FreeParameterExpression) and isinstance(
+                    param.expression, Expr
+                ):
                     free_params = param.expression.free_symbols
                     for parameter in free_params:
                         self._parameters.add(FreeParameter(parameter.name))

src/braket/parametric/free_parameter.py

@@ -47,7 +47,7 @@ def __init__(self, name: str):
             >>> param1 = FreeParameter("theta")
             >>> param1 = FreeParameter("\u03B8")
         """
-        self._name = Symbol(name)
+        self._set_name(name)
         super().__init__(expression=self._name)
 
     @property
@@ -87,6 +87,15 @@ def __repr__(self) -> str:
         """
         return self.name
 
+    def _set_name(self, name: str) -> None:
+        if not name:
+            raise ValueError("FreeParameter names must be non empty")
+        if not isinstance(name, str):
+            raise TypeError("FreeParameter names must be strings")
+        if not name[0].isalpha() and not name[0] == "_":
+            raise ValueError("FreeParameter names must start with a letter or an underscore")
+        self._name = Symbol(name)
+
     def to_dict(self) -> dict:
         return {
             "__class__": self.__class__.__name__,

src/braket/parametric/free_parameter_expression.py

@@ -107,7 +107,7 @@ def _parse_string_expression(self, expression: str) -> FreeParameterExpression:
         return self._eval_operation(ast.parse(expression, mode="eval").body)
 
     def _eval_operation(self, node: Any) -> FreeParameterExpression:
-        if isinstance(node, ast.Num):
+        if isinstance(node, ast.Constant):
             return FreeParameterExpression(node.n)
         elif isinstance(node, ast.Name):
             return FreeParameterExpression(sympy.Symbol(node.id))

src/braket/pulse/pulse_sequence.py

@@ -20,6 +20,7 @@
 
 from openpulse import ast
 from oqpy import BitVar, PhysicalQubits, Program
+from sympy import Expr
 
 from braket.parametric.free_parameter import FreeParameter
 from braket.parametric.free_parameter_expression import FreeParameterExpression
@@ -330,7 +331,9 @@ def _register_free_parameters(
         self,
         parameter: Union[float, FreeParameterExpression],
     ) -> None:
-        if isinstance(parameter, FreeParameterExpression):
+        if isinstance(parameter, FreeParameterExpression) and isinstance(
+            parameter.expression, Expr
+        ):
             for p in parameter.expression.free_symbols:
                 self._free_parameters.add(FreeParameter(p.name))
 

test/unit_tests/braket/circuits/test_circuit.py

@@ -21,6 +21,7 @@
     AsciiCircuitDiagram,
     Circuit,
     FreeParameter,
+    FreeParameterExpression,
     Gate,
     Instruction,
     Moments,
@@ -745,6 +746,44 @@ def test_ir_non_empty_instructions_result_types_basis_rotation_instructions():
     assert circ.to_ir() == expected
 
 
+@pytest.mark.parametrize(
+    "circuit, serialization_properties, expected_ir",
+    [
+        (
+            Circuit()
+            .rx(0, 0.15)
+            .ry(1, FreeParameterExpression("0.3"))
+            .rx(2, 3 * FreeParameterExpression(1)),
+            OpenQASMSerializationProperties(QubitReferenceType.VIRTUAL),
+            OpenQasmProgram(
+                source="\n".join(
+                    [
+                        "OPENQASM 3.0;",
+                        "bit[3] b;",
+                        "qubit[3] q;",
+                        "rx(0.15) q[0];",
+                        "ry(0.3) q[1];",
+                        "rx(3) q[2];",
+                        "b[0] = measure q[0];",
+                        "b[1] = measure q[1];",
+                        "b[2] = measure q[2];",
+                    ]
+                ),
+                inputs={},
+            ),
+        ),
+    ],
+)
+def test_circuit_to_ir_openqasm(circuit, serialization_properties, expected_ir):
+    assert (
+        circuit.to_ir(
+            ir_type=IRType.OPENQASM,
+            serialization_properties=serialization_properties,
+        )
+        == expected_ir
+    )
+
+
 @pytest.mark.parametrize(
     "circuit, serialization_properties, expected_ir",
     [
@@ -1019,7 +1058,9 @@ def test_ir_non_empty_instructions_result_types_basis_rotation_instructions():
         ),
     ],
 )
-def test_circuit_to_ir_openqasm(circuit, serialization_properties, expected_ir, gate_calibrations):
+def test_circuit_to_ir_openqasm_with_gate_calibrations(
+    circuit, serialization_properties, expected_ir, gate_calibrations
+):
     copy_of_gate_calibrations = gate_calibrations.copy()
     assert (
         circuit.to_ir(
@@ -1962,7 +2003,8 @@ def test_from_ir_inputs_updated():
                 source="\n".join(
                     [
                         "OPENQASM 3.0;",
-                        "input float theta;" "bit[1] b;",
+                        "input float theta;",
+                        "bit[1] b;",
                         "qubit[1] q;",
                         "rx(theta) q[0];",
                         "rx(2*theta) q[0];",

test/unit_tests/braket/parametric/test_free_parameter.py

@@ -21,9 +21,15 @@ def free_parameter():
     return FreeParameter("theta")
 
 
-@pytest.mark.xfail(raises=TypeError)
 def test_bad_input():
-    FreeParameter(6)
+    with pytest.raises(ValueError, match="FreeParameter names must be non empty"):
+        FreeParameter("")
+    with pytest.raises(TypeError, match="FreeParameter names must be strings"):
+        FreeParameter(6)
+    with pytest.raises(
+        ValueError, match="FreeParameter names must start with a letter or an underscore"
+    ):
+        FreeParameter(".2")
 
 
 def test_is_free_param(free_parameter):