Skip to content

Implement @warn, @error, ... without try/catch? #36174

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

Closed
schlichtanders opened this issue Jun 6, 2020 · 8 comments
Closed

Implement @warn, @error, ... without try/catch? #36174

schlichtanders opened this issue Jun 6, 2020 · 8 comments
Labels
logging The logging framework

Comments

@schlichtanders
Copy link

Trying out some autodifferentiation with Zygote I was surprised by code not being autodifferentiable because it made use of @warn macro.

Supporting try/catch in Zygote is an open issue however does not seem to be easily solvable in its whole generality any time soon.

I think supporting autodifferentiation in most parts of Julia is highly beneficial to the whole ecosystem. Hence I wanted to ask whether it is possible to rewrite @warn / @error and other standard Base logging macros to don't use try/catch.

@yuyichao
Copy link
Contributor

yuyichao commented Jun 6, 2020

I think supporting autodifferentiation in most parts of Julia is highly beneficial to the whole ecosystem.

So you are suggesting that try-catch be removed from "most parts of Julia"?

Selected most relavant functions might be OK though there is basically no replacement for exception handling so it may not be possible. It's unclear why does logging needs to be differentiatable though.

@KristofferC
Copy link
Member

This needs to be handled in Zygote. The try catches are useful in the logging macros.

@schlichtanders
Copy link
Author

schlichtanders commented Jun 6, 2020

Thank you very much for the information, I will push respectively on Zygote.

Logging just hit me, as some package somewhere within what I wanted to differentiate used @warn for logging... as simple as that

@tkf
Copy link
Member

tkf commented Jun 6, 2020

Probably the easiest short-term solution is to create a thin wrapper package that has macros like @warn s.t. @warn(args...) would be expanded to

__log__() do
    Logging.@warn(args...)  # maybe set `_module` etc. as well
end

with the definition __log__(f) = f(). You can then define a custom adjoint for __log__ to be a no-op.

@yuyichao
Copy link
Contributor

yuyichao commented Jun 6, 2020

Logging just hit me, as some package somewhere within what I wanted to differentiate used @warn for logging... as simple as that

Assuming the warning has no sideeffects and doesn't produce a result that you want to differentiate, if whatever method used cannot handle this then it really needs to be fixed...

@c42f
Copy link
Member

c42f commented Jun 7, 2020

Perhaps this may be fixed in IRTools because @warn has no side effects relevant to the differentiation FluxML/IRTools.jl#63 (mentioned by @Roger-luo on slack).

Having said that, I don't love the fact that @warn etc spam so much code into every function - maybe we can also do better there in the future. Originally we passed the log record generation around as a closure, but Jeff was concerned this led to excessive closure creation so that was removed.

However we could consider going back to outlining more of the logging code in the future; really the whole logging frontend needs a solid iteration of performance tuning and we could try it out then.

@MikeInnes
Copy link
Member

MikeInnes commented Jun 12, 2020

Agreed that we should fix this to the extent possible in Zygote as well. We have a PR for that in FluxML/Zygote.jl#466. Most likely we'll have to break if an exception is actually caught, though, since to support that correctly we'd have to undo the unwinding of the stack. (Anyone want to reimplement exceptions in terms of composable effect handlers, though?)

Assuming the warning has no sideeffects and doesn't produce a result that you want to differentiate, if whatever method used cannot handle this then it really needs to be fixed...

The first assumption is the problem there. Since we don't have type information (in general, even if we eventually work on typed IR) we have no idea what's side effecting and isn't without actually transforming it. So we won't be able to avoid this with something simple like an IRTools analysis.

@c42f
Copy link
Member

c42f commented Jun 12, 2020

Anyone want to reimplement exceptions in terms of composable effect handlers, though?

Yes. I was interested in effect handlers as part of a larger idea about unifying return codes and exceptions. Interesting that it also applies to exception handing in Zygote.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
logging The logging framework
Projects
None yet
Development

No branches or pull requests

6 participants