LpExtractor creates an infeasible problem for CBC #207
Comments
khaki3
changed the title
khaki3
changed the title
|
The code above does not support additional nodes. I ended up with this unpolished one: fn find_cycles<L, N>(egraph: &EGraph<L, N>, mut f: impl FnMut(Id, usize))
where
L: Language,
N: Analysis<L>,
{
let mut pending: HashMap<Id, Vec<(Id, usize)>> = HashMap::default();
let mut order: HashMap<Id, usize> = HashMap::default();
let mut memo: HashMap<(Id, usize), bool> = HashMap::default();
let mut stack: Vec<(Id, usize)> = vec![];
for class in egraph.classes() {
let id = class.id;
for (i, node) in egraph[id].iter().enumerate() {
for &child in node.children() {
pending.entry(child).or_insert_with(Vec::new).push((id, i));
}
if node.is_leaf() {
stack.push((id, i));
}
}
}
let mut count = 0;
while let Some((id, i)) = stack.pop() {
if memo.get(&(id, i)).is_some() {
continue;
}
let node = &egraph[id].nodes[i];
let mut update = false;
if node.is_leaf() {
update = true;
} else if node.children().iter().all(|&x| order.get(&x).is_some()) {
if let Some(ord) = order.get(&id) {
update = node.children().iter().all(|&x| order.get(&x).unwrap() < ord);
if !update {
memo.insert((id, i), false);
continue;
}
} else {
update = true;
}
}
if update {
if order.get(&id).is_none() {
order.insert(id, count);
count = count + 1;
}
memo.insert((id, i), true);
if let Some(mut v) = pending.remove(&id) {
stack.append(&mut v);
stack.sort();
stack.dedup();
};
}
}
for class in egraph.classes() {
let id = class.id;
for (i, node) in egraph[id].iter().enumerate() {
if let Some(true) = memo.get(&(id, i)) {
continue;
}
f(id, i);
}
}
} |
Great to hear! Thanks for providing a workaround that seems to work here. I'm not quite sure I fully understand what is deficient in the existing find_cycles. Do you? |
|
The current one starts the search from the top of |
|
My code is missing some optimal cases. The first one could be supported by checking actual cycles (not existing in this case so no remove) after constructing a RecExpr-like order. But the second one having interdependent cycles would need a cost function to choose but it's an expensive search.
use egg::*;
struct CostFn;
type EGraph = egg::EGraph<SymbolLang, ()>;
impl LpCostFunction<SymbolLang, ()> for CostFn {
fn node_cost(&mut self, _egraph: &EGraph, _eclass: Id, enode: &SymbolLang) -> f64
{
match enode.op.as_str() {
"A" => 1.0,
"B" => 2.0,
"x" => 10.0,
"y" => 3.0,
_ => 0.0,
}
}
}
fn main() {
env_logger::init();
let mut egraph = EGraph::default();
let a = egraph.add_expr(&"(A y)".parse().unwrap());
let x = egraph.add_expr(&"x".parse().unwrap());
let y = egraph.add_expr(&"y".parse().unwrap());
let top = egraph.add(SymbolLang::new("list", vec![a, y]));
egraph.union(a, x);
egraph.rebuild();
egraph.dot().to_dot("tmp1.dot").unwrap();
println!("{}", LpExtractor::new(&egraph, CostFn).solve(top)); // (list x y) while (list (A y) y) is optimal
let b = egraph.add_expr(&"(B x)".parse().unwrap());
egraph.union(b, y);
egraph.rebuild();
egraph.dot().to_dot("tmp2.dot").unwrap();
println!("{}", LpExtractor::new(&egraph, CostFn).solve(top)); // (list x (B x)) while (list (A y) y) is optimal
} |
|
Hmm, this seems tricky indeed. @Bastacyclop do you think this approach could solve #196? |
|
Hi @mwillsey , I'll investigate next week! |
|
Using @khaki3 's |
|
Using this solution on a broader set of benchmarks produced mitigated results for us: extraction times are increased (leading to timeouts), and extracted solutions are not always better. I'm currently attempting to implement an alternative solution, if you have ideas on benchmarks to evaluate on. |
Hi. I'm having fun with egg.
I encountered an infeasible error with CBC that prevents LpExtractor from processing cyclic e-graphs.
Example:
Log produced:
This
find_cyclesextractsgas a result. But I might needfinstead.https://github.com/egraphs-good/egg/blob/main/src/lp_extract.rs#L209
For the moment, I'm avoiding the error with code like
is_dag. Perhaps, it works for others.(Could be related: #196 )
The text was updated successfully, but these errors were encountered: