feature: Adding support for FreeParameterExpression #85
Conversation
|
Hi, @kshyatt-aws, I hope that we can complete the PRs and close the issue before the deadline! Thanks! I have also added a |
…s and subs function
|
Please checkout the subs of FreeParameterExpression in action! |
…dereview suggestions
…eneration example to doctests
|
Thanks a lot for your comments! Tested locally as well! |
|
Hi, @kshyatt-aws, Kindly Please check out the changes here as well! I hope you like it! |
|
Hi, @kshyatt-aws, A small reminder to please close the |
Fe-r-oz
changed the title
src/Braket.jl
Outdated
| FreeParameterExpression | ||
| FreeParameterExpression(expr::Union{FreeParameterExpression, Number, Symbolics.Num, String}) | ||
|
|
||
| Struct representing a Free Parameter expression, which can be used in symbolic computations. |
There was a problem hiding this comment.
FreeParameter here should get a doc link
There was a problem hiding this comment.
You might need to add this docstring to the relevant docs/src file explicitly
src/Braket.jl
Outdated
| FreeParameterExpression(expr) = throw(ArgumentError("Unsupported expression type")) | ||
|
|
||
| function FreeParameterExpression(expr::String) | ||
| parsed_expr = parse_expr_to_symbolic(Meta.parse(expr), @__MODULE__) |
There was a problem hiding this comment.
This looks very suspect to me... is there a less sketchy way to accomplish this?
There was a problem hiding this comment.
Generally directly calling Meta.parse can be pretty risky!
There was a problem hiding this comment.
I understand your concern. In order to alleviate it, Added a function validate_expr that checks for each char in string and make sure that error is thrown for suspect and unknown characters. This approach ensures that input expressions are strictly validated before parsing with Meta.parse.
Tried a bunch of different ways without Meta.parse. , but other approaches were errorsome. Checked out Link: https://github.com/JuliaSymbolics/Symbolics.jl/blob/master/src/parsing.jl and ExprTools as well.
# Function to validate the input expression string
function validate_expr(expr::String)
allowed_chars = Set("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789+-*/^()αβγδεζηθικλμνξοπρςστυφχψω ")
for char in expr
if !(char in allowed_chars)
throw(ArgumentError("Unsupported character '$char' in expression. Only ASCII letters (a-z, A-Z), Greek letters (α-ω), digits (0-9), space( ), and basic mathematical symbols (+ - * / ^ ()) are allowed."))
end
end
return expr
end
# Function to create FreeParameterExpression from a validated string
function FreeParameterExpression(expr::String)
validated_expr = validate_expr(expr)
parsed_expr = parse_expr_to_symbolic(Meta.parse(validated_expr), @__MODULE__)
return FreeParameterExpression(parsed_expr)
end
| function Base.:!=(fpe1::FreeParameterExpression, fpe2::FreeParameterExpression) | ||
| return !(isequal(fpe1.expression, fpe2.expression)) | ||
| end | ||
|
|
There was a problem hiding this comment.
Could slim all these down by making them one liners like:
Base.:^(fpe1::Union{Number, Symbolics.Num}, fpe2::FreeParameterExpression) = FreeParameterExpression(fpe1 ^ fpe2.expression)
There was a problem hiding this comment.
This was giving error
Base.:^(fpe1::Union{Number, Symbolics.Num}, fpe2::FreeParameterExpression) = FreeParameterExpression(fpe1 ^ fpe2.expression)
julia> result = fpe1 ^ fpe2
ERROR: MethodError: no method matching ^(::FreeParameterExpression, ::FreeParameterExpression)
Closest candidates are:
^(::Number, ::FreeParameterExpression)
@ Braket ~/Desktop/New/braket/2/Braket.jl/src/Braket.jl:211
^(::SymbolicUtils.Symbolic{<:Number}, ::Any)
@ SymbolicUtils ~/.julia/packages/SymbolicUtils/qyMYa/src/types.jl:1179
We can slim it down like this:
Base.:+(fpe1::FreeParameterExpression, fpe2::Union{FreeParameterExpression, Number, Symbolics.Num}) =
FreeParameterExpression((fpe1 isa FreeParameterExpression ? fpe1.expression : fpe1) +
(fpe2 isa FreeParameterExpression ? fpe2.expression : fpe2))
| gsub = subs(gate, Dict(:α => 2.0, :θ => 2.0)) | ||
| circ = Circuit() | ||
| circ = H(circ, 0) | ||
| circ = Rx(circ, 1, gsub) |
There was a problem hiding this comment.
But here gsub is still the FPE with the values subbed in. What happens if I do:
circ = Rx(circ, 1, gate)
?
There was a problem hiding this comment.
That will show the FPE before evaluating it via substitute at the backend wrapped as subs:
julia> circ = Circuit()
T : |Result Types|
T : |Result Types|
julia> circ = H(circ, 0)
T : |0|Result Types|
q0 : -H--------------
T : |0|Result Types|
julia> circ = Rx(circ, 1, gate)
T : | 0 |Result Types|
q0 : -H-----------------------
q1 : -Rx(α + 2θ)--------------
T : | 0 |Result Types|
|
OK, this looks good to let CI run! If it doesn't pass, that's ok, I can merge into a feature branch and the contribution will still count for UnitaryHack. |
|
I think approving the PR can also work since UnitaryHack website page for Maintainers says PR merged (or approved). This error was not occuring locally!
Anyhow, Thank you for the round of reviews! It's been fun working for the two packages! |
|
Were you running the whole workflow locally? You can try to do so with https://github.com/nektos/act to test the full CI pipeline. |
|
Please run this CI, I fixed the compatibility issue with Symbolics! I hope the CI passes now! |
Project.toml
Outdated
| @@ -64,6 +66,8 @@ SparseArrays = "1.6" | |||
| StaticArrays = "=1.9.3" | |||
| Statistics = "1.6" | |||
| StructTypes = "=1.10.0" | |||
| Symbolics = "=5.30.3" | |||
| SymbolicUtils = "2.0.2" | |||
There was a problem hiding this comment.
You might need an = here too
There was a problem hiding this comment.
Alright. I will do it like SymbolicUtils = "=2.0.2"
Resolving Issue #82
Description of changes: Replicate the Python SDK support for FreeParameterExpression, allowing gates like Rx( 2 * alpha /3), thus creating FreeParameterExpression
Note: New to git secrets, so bear with me please!
Testing done:
Local Testing
Merge Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
git secretsto make sure I did not commit any sensitive information (passwords or credentials) (not sure, I created gpg key and tried to initialize git-secret)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.