Closed TrevorHansen closed 6 months ago
Does this already combine with #24?
Does this already combine with #24?
Not yet. Hopefully when they're combined, and this ILP extractor is using good_lp/highs instead of CBC solver the runtime will drop by 1/3 (like @averyanalex has demonstrated with the current ILP extractor).
Going forward, I'm imagining something like this:
Sounds like a good plan!
Let me know if you are ever blocked on a review
Let me know if you are ever blocked on a review
Thanks @oflatt, I'm happy with this PR now, who should I ask to review it?
Does this already combine with #24?
Not yet. Hopefully when they're combined, and this ILP extractor is using good_lp/highs instead of CBC solver the runtime will drop by 1/3 (like @averyanalex has demonstrated with the current ILP extractor).
Going forward, I'm imagining something like this:
* Merge the good_lp/highs ILP extractor ([Add good_lp based (with highs solver) extractor #24](https://github.com/egraphs-good/extraction-gym/pull/24)) into main. * Merge this ILP extractor ([ILP-based extractor that produces an optimal dag-cost extraction #30](https://github.com/egraphs-good/extraction-gym/pull/30)) into main. * Merge the better test code ([Test code #31](https://github.com/egraphs-good/extraction-gym/pull/31)) into main. * Using the better test code, confirm that the updated extractors([Add good_lp based (with highs solver) extractor #24](https://github.com/egraphs-good/extraction-gym/pull/24), [ILP-based extractor that produces an optimal dag-cost extraction #30](https://github.com/egraphs-good/extraction-gym/pull/30)) are sound. * Change this extractor to use good_lp/highs
It might make sense to merge #30 first, and then I'll try to port the updated extractor to good_lp.
I like the overall plan, and I like the consolidation of extractor variants (too many makes it hard to compare). The problem with optimal ILP is cycle breaking, and I'm not 100% clear on how this approach works on that. Can you include a comment or something on how cycles are avoided? It seems like cycle avoidance strategies will be a major distinguishing factor for DAG-based extractors.
... The problem with optimal ILP is cycle breaking, and I'm not 100% clear on how this approach works on that. Can you include a comment or something on how cycles are avoided? It seems like cycle avoidance strategies will be a major distinguishing factor for DAG-based extractors.
Sure, I've added this description to the code:
To block cycles, we enforce that a topological ordering exists on the extraction. Each class is mapped to an integer variable (called its level). Then for each node, we add a constraint that if a node is active, then the level of the class the node belongs to must be less than than the level of each of the node's children.
To create a cycle, the levels would need to decrease, so they're blocked. For example, given a two class cycle: if class A, has level 'l', and class B has level 'm', then 'l' must be less than 'm', but because there is also an active node in class B that has class A as a child, 'm' must be less than 'l', which is a contradiction.
is it being an integer variable important? Could it be a real-valued one? That might be easier for the solver.
is it being an integer variable important? Could it be a real-valued one? That might be easier for the solver.
Nicely spotted! Combined with some other changes it reduced the runtime by about 15%.
I mentioned in #24 that I've made an updated ILP extractor that given enough time, will produce an optimal (least-cost) DAG-extaction. It's my preference to combine this and the work in #24, so we have a faster & simple ILP extractor that can produce optimal DAG extractions.
I now realise, either the bottom-up, or faster-bottom-up will produce an optimal tree-extraction. So with this ILP extractor we can now produce optimal dag or tree extractions.
I'm not sure if it's widely appreciated, but the current ILP extractor isn't good. Looking at the details below, you can see the the current/old extractor is about 95 times slower than the faster-greedy-dag extractor and produces extractions that are much worse than it. I can't see a reason to use the current/old ILP extractor.
The ILP solver in this PR is slow on some instances, with some taking >10 hours to optimise. So has a timeout, that if reached, will return the results from the faster-greedy-dag, which is fast and kinda-ok.
Here are the results at some timeouts for 260 test inputs.