-
Notifications
You must be signed in to change notification settings - Fork 33
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
Undo scheduler attempt #1004
Undo scheduler attempt #1004
Conversation
The unit tests are failing because I didn't update |
Ok, here's the branch results vs baseline. @paulzzy you can use the This might be because Herbie egraphs are quite small (8k nodes) so perhaps the undo scheduler just never gets a chance to fire, I don't know. |
Thanks for trying it out! I'll spend some time looking into what's going on |
I'm surprised the tests passed because I haven't yet implemented support for explanations (which suggests it wasn't actually running, so I'll figure out how to fix that). Are explanations required by Herbie? |
Herbie uses explanations but with them unimplemented I imagine you just return empty explanations? Which Herbie knows to ignore. |
Currently it just panics with |
Oh. Huh, then I wonder what this PR even did. Do you want to take a look at teh code? The egg side is tiny / trivial. |
Currently taking a look! Looks like it panics without explanations being enabled, so I'm figuring out how to bypass that |
@paulzzy I'm going to close this PR, since it's not going to be merged and to keep our PRs clean, but please feel free to fork & make PR when you're ready on your end. |
Thanks Pavel! I'll submit a PR once I get it working. |
Test https://github.com/paulzzy/egg in Herbie. This is the "Undo Scheduler" described on Zulip which should make egg faster and hopefully also better.