-
Notifications
You must be signed in to change notification settings - Fork 19
ProgressLogging default environments #18
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
Comments
Actually, IMO detecting the environment and using a specific logging viewer is against the purpose of ProgressLogging. The purpose of ProgressLogging is to provide an easy, backend-agnostic way of defining progress logging with the help of the Julia logging system. Users then can decide if they, e.g., want to drop the logging messages, archive them to files, or display them in the terminal. That being said, Juno will display a native progress bar automatically. For displaying progress bars in the REPL, users can install an appropriate logger such as using TerminalLoggers: TerminalLogger
using Logging: global_logger
global_logger(TerminalLogger()) or only locally when calling using TerminalLoggers: TerminalLogger
using Logging: with_logger
with_logger(TerminalLogger()) do
sample(....)
end If In principle, this also works with Jupyter notebooks, however, the output of |
See also JuliaLogging/TerminalLoggers.jl#25 |
One of the big reasons we provided |
We could have something like function sample(. . .; logger=TerminalLogger(right_justify=120))
. . .
end And then there's a nice default for those who don't care enough to set this moderately arcane setting in their startup file, and those who want special loggers can just define what they want. |
I think the correct approach would be to increase the log level such that the progress is always displayed as a regular log message as a fallback, in case this is desired. The advantage of using the logging system is that it supports multiple backends and environments (such as Juno and REPL), but by providing a default logger we interfere with user-defined setups in a way that removes this advantage completely. I'm not even sure how you would have to define the logger if you are using Juno, and in general anyone who does not want to use |
We could also just check whether the current logger is the default logger, so we don't ignore people who've got their own setup: if Loggers.global_logger() isa Loggers.ConsoleLogger
local_logger = TerminalLogger()
end I think it's worth spending time on figuring how Juno should fit into this. I think showing progress bars is pretty damn close to a requirement for something like Turing -- it's used in basically every other PPL with zero work on the part of users. Having a progress bar is a strong quality signal -- spitting out log messages is a very tacky thing to do for what is essentially the only responsive UI element anybody who uses Turing is exposed to. |
This is prone to errors and almost guaranteed to break (e.g., if Juno is used; someone has changed some settings such as the log level of the standard logger on purpose and hence actually wants to use it; and more importantly someone has set a local logger such as a ConsoleProgressMonitor in a Jupyter notebook that should not be replaced with a TerminalLogger), and hence IMO there is no good reason for trying to be clever at this point. The most natural and intended setup is to let the user pick whatever backend they want to use (which is even set up automatically if they are using Juno).
First of all, I think in AbstractMCMC the focus should be on defining generally applicable sensible defaults without concentrating on Turing only. To me, changing the logger is definitely not a sensible default setting, and hence should be done at most in Turing itself. However, as explained above, IMO not even Turing should mess around with the loggers in whatever more or less clever way since IMO this would be unintended and unexpected behaviour when calling In my opinion, it is not unreasonable to expect from users to use and load custom loggers if they do not use Juno and want to print a progress bar in the REPL - as mentioned above, this can be done even automatically by modifying the Julia config file. Turing requires you to implement your probabilistic models anyway, so why should it be problematic if a user has to add two more lines that load ConsoleProgressMonitor or TerminalLoggers, as explained in the documentation? JuliaDiffEq and Flux use the same approach, so why should it be different in Turing? If the default output should be more verbose on the REPL to provide visual feedback also to users that do not set a specific logger (I'm not sure if that's actually a good idea), then increasing the loglevel would e.g. result in output such as ┌ Info: Sampling
└ progress = 0.94
┌ Info: Sampling
└ progress = 0.95
┌ Info: Sampling
└ progress = 0.96
┌ Info: Sampling
└ progress = 0.97
┌ Info: Sampling
└ progress = 0.98
┌ Info: Sampling
└ progress = 0.99
┌ Info: Sampling
└ progress = 1.0
┌ Info: Sampling
└ progress = "done" |
I want to be quite clear here -- I agree with you that what I am proposing is not ideal from an engineering standpoint. A perfect system filled with sophisticated software people would just require all of the downstream users to set their own loggers. That would be great, and it's a fine idea if that is who all of our users were. But that is not the case. What you are proposing here is essentially a regressive tax on our most inexperienced users, who are often new to Julia. None of these users are likely to touch their We do not offer just another package. If that were the case, sure, whatever, set your own logger. Turing is a comprehensive suite of statistical analysis tools, and it should behave as such! That means we should make an effort (what I view as a very small effort) to provide incredibly rudimentary features like progress bars or telling people when they have numerical errors. We want the statisticians who are not really the best programmers as much as we want the 1% of stats people who also happen to be Julia experts. If that means we have to pull away from the Flux/DiffEq world a bit, I am more than happy to do so. If I'm being honest, I think this is one thing that the ecosystem has wrong -- progress meters are essentially the most basic way to signal quality. If this were Python or R (or anything else that faces users), we would likely not ever have this discussion -- everyone would just agree that the package should write a very small amount of code to say "Hey, I know you asked me to do that thing, and I am now doing it. Hold tight". I'm fine with ProgressLogging if it just dispatches to the appropriate logging environment. If we're in the REPL, it should go to TerminalLoggers. If we're in Jupyer, it should go to ConsoleProgressMonitor. If we're in Juno, it's fine. If someone has anything other than the default ConsoleLogger, we should give them the most useful way to show progress. But it doesn't do that! It just throws all these messages into a blank pit by default, and we assume that users will "just know" that they have a way to get those messages out of the pit. I don't know about you, but when I first learned programming, and even when I learn a new language, I do not know that there is a pit, that all my messages go into that pit, and that there's a way to get stuff out of the pit. Sure, we could just stick
I think progress meters are a generally applicable default in this setting, particularly for all MCMC methods. That's why progress monitoring was included in the first place -- everyone wants to know how far along their sampling is and they may not have the know-how to add the additional line to their startup file, or to every single script they write. At the very least, this should be a default for Turing, if not for AbstractMCMC. I'm fine with backing off here if I am strongly in the minority, but I think we should keep an eye on the newest members of Julia. Every time we make something more engineer-friendly without providing a useful default for the less sophisticated, we introduce yet another friction to someone who is brand new to Turing and to Julia. We do not have to be some ultra-sleek DIY package that is completely free of defaults that make the lives of the uninitiated easy. The whole point of all of this is to make things easy! We wouldn't write an interface package, or Turing, or AdvancedHMC, or any of the other tools we provided if everyone just rolled their own stuff. If it's low cost and trivial it should just be a part of the system. |
I completely agree that we should help inexperienced Julia users and provide them with a good user experience. Sorry if that was not clear from my comments above. I'm still wondering how we could get the best of both worlds - providing a reasonable default without any configuration and not interfering with the logging system too much. As far as I see, we could check the combination of the user environment and the current logger, and if they seem unsuitable for progress logging
The first approach
The second approach
Somewhat in between these two solutions would be the approach to
This approach
Even though it is not perfect, I think the third approach could provide a reasonable trade-off between providing every user with some feedback about the progress of sampling and avoiding any potentially unintended hacks. |
More concretely, without any additional dependencies we use something along the lines of function isprogresslogfiltered()
# assume Juno works fine
isdefined(Main, :Atom) && return false
# otherwise check if progress logs are filtered
logger = Logging.current_logger()
Logging.min_enabled_level(logger) < ProgressLogging.ProgressLevel && return false
# display warning and add instructions for how to install a backend
if isdefined(Main, :IJulia)
@warn "Progress bars can not be displayed. Please install a progress logging frontend such as [ConsoleProgressMonitor.jl](https://github.com/tkf/ConsoleProgressMonitor.jl)."
else
@warn "Progress bars can not be displayed. Please install a progress logging frontend such as [TerminalLoggers.jl](https://github.com/c42f/TerminalLoggers.jl)."
end
return true
end The return value could be used to update |
OK, as I expected initially, such a simple check won't work. The problem with the code above is that neither the current logger nor the global logger need to be able to display the progress logs, it would be sufficient if any logger in the dynamical stack of loggers is suitable - but that's something we can't (?) check. According to the documentation we also shouldn't try to do it, since
So another idea would be to abuse |
Couldn't we just have function sample(. . .;
logger=global_logger() == Logging.ConsoleLogger(stderr) ?
TerminalLogger :
global_logger()
)
with_logger(logger) do
# sampling stuff
end
end In this case we check if the global logger is anything other than the default (set here). If someone's set it to something else, we use that. If not, we add |
That test is not reliable since it is not unlikely that one uses TerminalLogger or some other custom logger as a task-specific logger (and also Juno does not change it). I think there is no better way to check the availability of loggers than checking if the packages were loaded (assuming that an inexperienced user might not load them). |
Well, |
No, unfortunately it's not an alternative, it does not work for the very same reason. The logging system by design allows an extreme amount of composability, so that you can run some parts of your code with a TerminalLogger in place but still handle, e.g., I still believe, if our main concern are inexperienced users that do not know about Julia's logging system, our best guess is to assume that these users have problems since they do not load ConsoleProgressMonitor or TerminalLoggers. I think that's a quite reasonable assumption, and hence at the moment I'm tending towards just checking if one of these packages was loaded, and presenting a warning otherwise. However, maybe there are some issues with this approach as well that I don't see right now 🤷♀️ All in all, I'm very much in favour of presenting a warning/explanation/help/fix to new users but I strongly believe that in a package such as AbstractMCMC that is supposed to be reused and extended by many other packages that has to be done in a way that does not break the code of people that use the logging system correctly in mysterious and unexpected ways (such as, e.g., a keyword argument would). |
I think there is just some very large point here that I am not seeing, and I think I should drop this whole issue. I don't have a clear picture in my head of why any of this was implemented, though obviously there are many reasons which you have been kind enough to outline in this thread. I appreciate that you've taken the time to explain this all to me in great detail. Can you add a section about how to get progress bars to show up to the Turing docs? |
There's a rudimentary section but I guess we should extend that and maybe provide some examples? |
Yeah, it should have a block to show how to use it in Jupyter and the terminal separately, so people can just copy the code. |
I think we can close this for now, hopefully we came up with a reasonable solution. |
Yeah, I think what you've got is quite elegant. |
I just noticed that the new ProgressLogging.jl backend does not actually have any display functionality by default, and that downstream users do not get any automatic progress bar updates.
Is there a way to detect which environment someone is in (either Juno or a terminal) and to use the appropriate progress viewer automatically? I wasn't able to find a good way to do so.
The text was updated successfully, but these errors were encountered: