-
Notifications
You must be signed in to change notification settings - Fork 11
Add CachedExpressionConvertible #72
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
Conversation
3b66a9c
to
ee777e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from a docstring request, LGTM. Handing over to @jcjaskula-aws for braket perspective.
@@ -62,6 +62,7 @@ | |||
"ComplexVar", | |||
"DurationVar", | |||
"OQFunctionCall", | |||
"OQIndexExpression", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am sneaking in one unrelated change in this PR. I see that previously this was updated (cf 862d0c1), but OQIndexExpression
was not actually moved. It should be exported as before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any reason right now why this should be a problem for Braket. Would there be any drawback to do this with OQPyExpression instead of creating a new protocol?
I had a vague concern about backwards compatibility here. One could very well imagine somebody constructing a program in which they mutate an |
This adds a new protocol,
CachedExpressionConvertible
, which can be used analogously toExpressionConvertible
but with support for the constructed expression to be reused during program construction. This can improve performance during construction of programs in which an underlyingExpressionConvertible
object be used many times, or when the actual work of AST construction is non-negligible.