automlbenchmark icon indicating copy to clipboard operation
automlbenchmark copied to clipboard

Logger defaults to printing to stderr instead of stdout

Open eddiebergman opened this issue 3 years ago • 6 comments

While using automlbenchmark on a slurm cluster, specifying a specific out and error stream, I noticed automlbenchmark outputs all logging to stderr.

Reproduce

python runbenchmark -s only flaml > out.txt 2> err.txt
cat out.txt # Empty
cat error.txt # All the normal output

I'm not sure how logging should be configured overall in terms of what gets printed to stdout and stderr but I've left a simple change here in case you'd like to change it in the short term.

Short fix

# amlb/logger.py:55, def setup(...): ...
console = logging.StreamHandler(stream=sys.stdout)

Here's the constructor just to show that id defaults to sys.stderr

class StreamHandler:
    def __init__(self, stream=None):
            """
            Initialize the handler.
    
            If stream is not specified, sys.stderr is used.
            """
            Handler.__init__(self)
            if stream is None:
                stream = sys.stderr
            self.stream = stream

eddiebergman avatar Oct 02 '21 12:10 eddiebergman

I'm not sure why stderr is configured as default output stream. @sebhrusen?

PGijsbers avatar Oct 03 '21 12:10 PGijsbers

It's Python core library's decision to log to stderr by default. It's common for logs to be redirected to stderr as stdout is usually reserved for a program's output/result: e.g. calls to print() are usually considered as proper output, and separated from logs.

sebhrusen avatar Oct 03 '21 13:10 sebhrusen

@eddiebergman instead of fixing the logger, it rather looks like you expect the result scores to be printed to stdout, which is a fair expectation, we can add a print() statement for this.

sebhrusen avatar Oct 03 '21 13:10 sebhrusen

Hi @sebhrusen,

While yes they default to stderr, i generally expect a program to only output to stderr when an error of some kind has occurred, where the program has exited with a failed state. For example, if the framework failed to setup, for the error message to be put there with the general setup output going to stdout. I find it an odd choice that python's logging StreamHandler defaults to stderr.

However I would think this is not so simple given the various other possible logging event's like 'debug' and 'warning', I don't really know where I would expect them to go, perhaps stdout as they are not explicitly errors.

Please feel free to continue with this how you wish, I just wanted to bring this to your attention :)

eddiebergman avatar Oct 03 '21 16:10 eddiebergman

@eddiebergman, yes, there are basically 2 common practices for command line apps. A common one—who lead to Python's decision—is to redirect logs to stderr and reserve stdout for result or any "useful output"; the rationale being that this way,stdout can easily be pipped to other commands, usually only interested in the result. Actually,stderr is supposed to handle all diagnostics, not only errors, and, imo, it makes sense to consider all logs as diagnostics. Another is to consider that stderr should get content only in case of errors. I think it's perfectly fine for short-running programs on the CLI, but long running ones like this one need logs, and splitting console logs, depending on their levels, between stdout and stderr would be even more confusing. Personally, I think that the exit code should be source of truth to decide if a program ended normally or not, not if stderr has any content. And in case of a relatively complex app like this one, where a single run can trigger multiple jobs, some of them passing, some others failing, you can have a program ending normally but with a couple of internal jobs failing, which is very different from a complete failure (in this case, it's not the app itself that failed, it's one/some of the framework's task).

I'll have a look at what we can do and will try to print all—and only—the benchmark results to stdout. I think this will make a consistent usage of both streams, according to the first common practice mentioned above.

Thanks for raising this, I didn't pay much attention to it before, will definitely try to improve it.

sebhrusen avatar Oct 03 '21 19:10 sebhrusen

Hi @sebhrusen,

Thanks for the detailed response and your rationale, this all makes sense and I agree with your decisions :) I also agree that only the end results should be piped to stdout in this case. I don't see many cases where this would be used but it's at least in line with other programs in that case.

Feel free to close this issue if you would like!

eddiebergman avatar Oct 03 '21 19:10 eddiebergman