Generic analysis

[dewert99]

Note this assumes #291
I've noticed that EGraph and EClass struct's can be broken down into various parts that serve different purposes:

• Core egraph: (EGraph.{unionfind, nodes, memo, pending, classes}, EClass.{id, parents})

  • Enough to support basic egraph operations like adding nodes, performing unions, checking equivalence, and restoring congruence closure (via process_unions)
  • Sufficient to build DOT visualizations of the egraph using union-find and nodes
  • EGraph.parents could potentially be de-duplicated but this is never required

• Backtracking e-matching: EGraph.classes_by_op, EClass.nodes

  • Could be constructed from core egraph, but EClass.nodes is maintained partially incrementally
  • Requires being rebuilt using rebuild_classes before e-matching
  • Probably not necessary if a different e-matching strategy were used (eg. generic join)

• Custom Lattice Analysis: EGraph.{analysis, analysis_pending}, EClass.data

  • Since this is a type parameter it can be almost zero-cost by instantiating N=()
  • Still has slight overhead of keeping EGraph.analysis_pending when Analysis::merge always returns DidMerge(false, false)

• Explanations: EGraph.explain

  • Toggled dynamically using an Option causing slight overhead
  • Controls whether a new node that is equivalent to an existing node (by congruence), but not identical to any existing node, gets a new Id
  • Requires explanations to be Symbols

I've started experimenting with trying to factor out the core egraph and use a new Analysis trait (I renamed the old one to LatticeAnalysis) to handle everything else though so far the core egraph still includes explanations. This Analysis trait is implemented for pairs of Analysiss to allow composing multiple together. I created to implementation of this Analysis, trait EMatchingAnalysis which contains classes_by_op and has EClass.nodes as its data field, and WrapLatticeAnalysis that wraps a LatticeAnalysis to handle analysis_pending.

To handle methods that relied on a specific analysis like set_analysis_data or search_eclass I implemented them for egraphs with specific analysis instantiations, eg EGraph<L, (WrapAnalysisData<N>, N2)> for set_analysis_data or EGraph<L, (N, EMatchingAnalysis<L>)> for search_eclass, and I created the alias legacy::EGraph<L, N> = EGraph<L, (WrapAnalysisData<N>, EMatchingAnalysis<L>)>. While this has worked fine, for now, it seems like it will break down if users want to use these types of methods for multiple analyses at once, for example, if the explain_equivalence required something like EGraph<L, (ExplainAnalysis, N2)> then a user could call it and set_analysis_data on the same EGraph. I could have something like implement set_analysis_data for EGraph<L, ((WrapAnalysisData<N> N2), N3)> and implement explain_equivalence for EGraph<L, ((N2, ExplainAnalysis), N3)> but this doesn't seem scalable.

Other side effects of this change EGraph::dot, EGraph::intersect, and Extractor::new are more expensive since they need to calculate the nodes in each eclass, and Analysiss can't access the nodes of an eclass. This second change prevented the constant folding analysis from the math test from removing nodes from eclasses representing constants which caused 4 of the tests to fail.

I'm making this a draft PR so that you can view the exact changes I made but I am mostly interested in discussion

[mwillsey]

Just so I understand; this is not a user facing change? The goal is to implement various egg internals using a more flexible analysis API, right?

[dewert99]

Unfortunately, the old Analysis trait had methods with EGraph<L, Self> parameters which makes it difficult to use compositionally. I ended up switching these parameters to impl EGraphT<L, N=Self> where EGraphT derefs to an egraph (although not necessarily one with Self as the analysis, and has methods for converting the egraphs analysis to the analysis being implemented (and similar for the analysis's data).

[mwillsey]

I see. I think this is a cool idea, and could be the right thing going forward. I would hope that it would result in code reduction rather and growth, but maybe that's asking for too much. In general, I'm pretty hesitant (but not totally unwilling) to break the user-facing API at this point unless there is some demonstrable need. Most of the egg maintainers (including me) are spending more time on other projects at the moment. But don't let that stop you from experimenting or working on a fork!

[dewert99]

@robert-chiniquy I heard that you were working on a fork of egg, and thought these changes might be interesting to you.

[robert-chiniquy]

Oh, heh, sorry, I'm absolutely not going to work on a fork, I'm just going to use my own hacked fork until something equivalent to my latest PR lands (this is how I avoid waiting).