[WIP] Add ParameterExpression in Rust #13278
Conversation
|
One or more of the following people are relevant to this code:
|
Pull Request Test Coverage Report for Build 12024667837Details
💛 - Coveralls |
@Cryoris can confirm, but I think on main we don't have to worry about this for 2.0 since we're dropping pulse support. AFAIK the complex parameter binding and handling was only necessary for pulse expressions. But maybe there is a use case I'm overlooking. |
|
For circuits, real is enough. But we currently also allow parameter expressions in the |
1ucian0
added
the
Rust
| rhs : SymbolExpr, | ||
| } | ||
|
|
||
| impl Clone for SymbolExpr { |
There was a problem hiding this comment.
Why do we need this separate Clone implementation? It looks like it would be the same as the standard Clone, but maybe I'm missing something. Or is this doing something specific to the Arc in Unary and Binary?
| Value::Real(r) => r, | ||
| Value::Complex(c) => c.re, | ||
| } | ||
| None => 0.0, |
There was a problem hiding this comment.
I might not understand this fully, but if self is a plain Symbol then self.eval returns None and this the functions real/complex will return 0, right? But if I just have a plain Symbol with no value associated yet, then I would expect that I cannot query the real or complex parts, because they are not defined, no? (In which case I thought this might need to raise an error 🙂)
| } | ||
| } | ||
|
|
||
| pub fn has_symbol(&self, param: String) -> bool { |
There was a problem hiding this comment.
Should we use &String or even &str here since we're only reading the value?
| _ => false, | ||
| }, | ||
| SymbolExpr::Unary(e) => e.expr.is_complex(), | ||
| SymbolExpr::Binary(e) => e.lhs.is_complex() || e.rhs.is_complex(), |
There was a problem hiding this comment.
Could this get us into trouble in expressions like 1j * 1j, i.e. where LHS and RHS are complex but the expression is not? (Same question for is_real)
| match self.lhs.eval(recurse) { | ||
| Some(v) => lval = v, | ||
| None => return None, | ||
| } | ||
| match self.rhs.eval(recurse) { | ||
| Some(v) => rval = v, | ||
| None => return None, | ||
| } |
There was a problem hiding this comment.
You could make this a bit less typing by matching the tuple if you would like (no guarantees on the formatting 🙂)
| match self.lhs.eval(recurse) { | |
| Some(v) => lval = v, | |
| None => return None, | |
| } | |
| match self.rhs.eval(recurse) { | |
| Some(v) => rval = v, | |
| None => return None, | |
| } | |
| match (self.lhs.eval(true), self.rhs.eval(true)) { | |
| (Some(left), Some(right)) => { | |
| lval = left; | |
| rval = right; | |
| } | |
| _ => return None, | |
| } |
Summary
This is draft of ParameterExpression in Rust for #13267
Details and comments
I made symbolic algebra engine instead of using symengine to remove dependencies of conan.
The functionalities are limited compared to symengine but I'm implementing enough functions for ParameterExpression
[TO DOs]