-
Notifications
You must be signed in to change notification settings - Fork 85
Extraction as a top-level command in the frontend for the new backend #571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
no action extraction in progress
Alex-Fischman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not reviewed the actual extraction algorithm. My comments are about style and software engineering.
oflatt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did a quick and rough review
oflatt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
oflatt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, saw an opportunity
oflatt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
never mind, the external function is necessary
Alex-Fischman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of my comments have been resolved
This PR implements extraction as a top-level command in the frontend. It uses the default additive tree cost model and can only be triggered through
(extract [term] [variant]).Implementation overview:
query-extractas a top-level command has also been removed.Extractis now aCommandand is interpreted in the frontendrun_command.Extractfirst runs two backend queries to resolve the eclass and the number of variants to be extracted and then callsExtractorAlterto perform the extraction.ExtractorAlterfirst needs to be initialized by passing the list of rootsorts to be extracted. It computes all relevant costs using Bellman-Ford and remembers the necessary information for reconstruction in this step. Then, it can be queried multiple times to reconstruct the terms in termdags. It does not hold a reference toEGraph, so it can be reused as long as the relevant tables do not change.ExtractorAlterfirst performs a BFS on the sorts to filter out tables unreachable from the root sort.ExtractorAlterenumerates all rows again to copy information for reconstruction. Ties are broken arbitrarily.Additional remarks:
Extractorto this new architecture. So, no extraction in the old backend after this PR.