egraphs-good / egglog

egraphs + datalog!
https://egraphs-good.github.io/egglog/
MIT License
459 stars 54 forks source link

Report errors to stdout in REPL mode #417

Open yihozhang opened 3 months ago

yihozhang commented 3 months ago

Without this PR, the user of egglog (which can be say another process) cannot check if the command is successfully executed. The downside of this change is that every error message is now duplicated in the REPL mode. I don't know what the best design here would be. Maybe having another command line flag for this?

Indeed, there are some nuanced differences in the expected behaviors of REPL-mode egglog vs egglog communicating with other processes via stdin/out ("sidecar" egglog).

Thoughts and ideas are welcome!

saulshanabrook commented 3 months ago

I didn't know sidecar egglog was a thing!

If a command fails, REPL egglog should just ignore this command and continue to read second command, while it may make more sense for sidecar egglog to halt.

That makes sense to me.

In REPL egglog because both stderr and stdout are printed in the CLI, we should avoid duplicating information like error messages between stdout/stderr. But this is not true for sidecar egglog since the host process only monitors stderr.

Hm, why doesn't it monitor stdout?

yihozhang commented 3 months ago

Oops, I meant in the sidecar egglog the host only monitors stdout (not stderr, which are just logging information that is hard to parse).

I didn't know sidecar egglog was a thing!

I’m trying this out for my current project just to see how robust egglog is if the user only wants to use it as an external solver without writing any Rust (or Python). We will see how it goes :)

On Tue, Aug 20, 2024 at 8:19 AM Saul Shanabrook @.***> wrote:

I didn't know sidecar egglog was a thing!

If a command fails, REPL egglog should just ignore this command and continue to read second command, while it may make more sense for sidecar egglog to halt.

That makes sense to me.

In REPL egglog because both stderr and stdout are printed in the CLI, we should avoid duplicating information like error messages between stdout/stderr. But this is not true for sidecar egglog since the host process only monitors stderr.

Hm, why doesn't it monitor stdout?

— Reply to this email directly, view it on GitHub https://github.com/egraphs-good/egglog/pull/417#issuecomment-2299118530, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADEKPFBX4N3POH3SD46WCP3ZSNM6TAVCNFSM6AAAAABMY3RL2SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJZGEYTQNJTGA . You are receiving this because you authored the thread.Message ID: @.***>

oflatt commented 3 months ago

For sidecar egglog, I think this PR isn't sufficient. We need a reliable way to check that a command has finished, since the absence of an error isn't enough.

My recommendation is that we have an option that prints s-expressions as the output, one s-expression per command. So in sidecar mode we would have ( "Added a rule with name myrule to ruleset blank" "Some other information printed out" )

As for the duplication, having a command line flag for REPL mode sounds fine to me

oflatt commented 3 months ago

@ezrosent suggestion: Give egglog a mode where it accepts egglog and spits out JSON. That way you get structured output. @oflatt likes this idea (the default could be REPL mode, solving the problem in this PR)