feat: Circuit cost module and methods #165
Conversation
src/circuit/cost.rs
Outdated
|
|
||
| /// The cost for a group of operations in a circuit, each with cost `OpCost`. | ||
| pub trait CircuitCost: Add<Output = Self> + Sum<Self> + Debug + Default + Clone + Ord { | ||
| /// Returns true if the cost is above the threshold. |
There was a problem hiding this comment.
| /// Returns true if the cost is above the threshold. | |
| /// Returns true if the cost is above the threshold. | |
| /// | |
| /// It must hold that if `a.check_threshold(b)` then `a > b` but the reverse is not necessarily true. |
Is there a better name for this? I find gt_plus_epsilon an ugly but somewhat more descriptive name of what we want...
There was a problem hiding this comment.
I went with a sub_cost(self, rhs: Self) -> isize method, since it doesn't make sense for distance to return a negative number.
src/rewrite/strategy.rs
Outdated
| @@ -44,6 +45,9 @@ pub trait RewriteStrategy { | |||
|
|
|||
| /// The cost of a circuit. | |||
| fn circuit_cost(&self, circ: &Hugr) -> Self::Cost; | |||
There was a problem hiding this comment.
| fn circuit_cost(&self, circ: &Hugr) -> Self::Cost; | |
| fn circuit_cost(&self, circ: &Hugr) -> Self::Cost { | |
| circ.circuit_cost(|op| self.op_cost(op) | |
| } |
Does this not work?
| fn circuit_cost(&self, circ: &Hugr) -> Self::Cost { | ||
| circ.num_gates() | ||
| } |
There was a problem hiding this comment.
If you can add the default implementation above, you should be able to remove this.
There was a problem hiding this comment.
In this case in particular the overload is better, since it doesn't require a node traversal.
src/rewrite/strategy.rs
Outdated
| /// Whether the rewrite is allowed or not, based on the cost of the pattern and target. | ||
| fn threshold(&self, pattern_cost: &Self::SumOpCost, target_cost: &Self::SumOpCost) -> bool; | ||
| /// Returns true if the rewrite is allowed, based on the cost of the pattern and target. | ||
| fn under_threshold(&self, pattern_cost: &Self::OpCost, target_cost: &Self::OpCost) -> bool; |
There was a problem hiding this comment.
I think here again you should be able to give a default implementation, given that you have the bound OpCost: CircuitCost
src/rewrite/strategy.rs
Outdated
| @@ -150,7 +153,11 @@ where | |||
| } | |||
|
|
|||
| fn circuit_cost(&self, circ: &Hugr) -> Self::Cost { | |||
| cost(circ.nodes(), circ, |op| self.op_cost(op)) | |||
| circ.nodes_cost(circ.nodes(), |op| self.op_cost(op)) | |||
There was a problem hiding this comment.
Is this different from circ.circuit_cost()?
|
|
||
| fn threshold(&self, &pattern_cost: &Self::SumOpCost, &target_cost: &Self::SumOpCost) -> bool { | ||
| fn under_threshold(&self, &pattern_cost: &Self::OpCost, &target_cost: &Self::OpCost) -> bool { | ||
| (target_cost as f64) < self.gamma * (pattern_cost as f64) |
There was a problem hiding this comment.
I guess here a default impl would give the wrong thing, but you could just override it.
|
Sorry to get back to this, but IDEA: What if in fn distance(&self, other: &Self) -> isize/f64 |
aborgna-q
force-pushed
the
feat/circuit-cost
branch
from
ec1612d to
f45389a
Compare
Extracts the circuit cost definitions from
rewrite::strategyintocircuit::cost, and adds methodsCircuit::nodes_costandCircuit::circuit_cost.With this we can use the cost logic for
CircuitChunks::split_with_cost.Drive-by: There are some
CircuitChunksQoL updates extracted from #149 that I didn't separate from this PR.