Assertions etc. about k2 HOT 27 CLOSED

GLOG (we have used it in our code) supports Severity Level (INFO, WARNING, ERROR, and FATAL). However, I think maybe we should wrap it into a custom class as we may need to handle some exceptions from other frameworks (e.g. PyTorch, TensorFlow) or pass errors (from our assertions) as exceptions/error_code to other frameworks?

from k2.

from k2.

from k2.

Well, with GLOG, we can write code like this CHECK_GE(arc.label, -1) << "Arc label must be greater than -1";, i.e. assertion with logs. But we definitely need to wrap it to add more informations (e.g. file name, function name, line number, etc.), maybe some assertion mechanism like Kaldi does?

https://github.com/kaldi-asr/kaldi/blob/master/src/base/kaldi-error.h#L164-L165

from k2.

from k2.

Not sure whether the debug macros from https://github.com/NVlabs/cub/blob/master/cub/util_debug.cuh are useful.

from k2.

from k2.

In that case, the following files from bazel might be a good start:

• https://github.com/bazelbuild/bazel/blob/master/src/main/cpp/util/logging.h
• https://github.com/bazelbuild/bazel/blob/master/src/main/cpp/util/logging.cc

from k2.

from k2.

We do not need IsOk from the above code. What I wanted to show is that it's straightforward to implement
something like CHECK_EQ and LOG(INFO)

from k2.

Hi guys, I would like to do this.

from k2.

from k2.

K2_CHECK_EQ

Whether or not the API is like this, is related to how we do this. With glog, keep the same API is necessary, to make use another customized logger.

The pytorch and tf both implement a warpper of glog (to be more convenient or detailed).

https://github.com/pytorch/pytorch/blob/2912390662c3c50732add54c94ac9b98d17e1cba/c10/util/Logging.h#L24
https://github.com/tensorflow/tensorflow/blob/44597e39fb9e6eeb614d2119d516c0d4ede084cc/tensorflow/core/platform/logging.h#L22

They have glog as one preferred logger, and a customized one with same API as glog. One is chosen based on macros (related to the platform, glog availability, ..)

Thoughts:

• To avoid the macro or version conflict that glog may introduce, and consider the lightweight needing currently (w/o file rotation, concurrence, performance), I would advise a customized one like KALDI_ASSERT implementation, or lightweight/header-only ones (e.g. pytorch/tf customization, or https://github.com/gabime/spdlog).

• And in our case that is to make a cpp extension for pytorch, why not just use what we need from the customized logging header of pytorch for now. By this way, we keep glog around, and could extend it in two similar customized headers:

  • one for cover the common API of glog
    https://github.com/pytorch/pytorch/blob/2912390662c3c50732add54c94ac9b98d17e1cba/c10/util/logging_is_not_google_glog.h
  • the other to check a macro, include glog, and extension API independent with the macro.
    https://github.com/pytorch/pytorch/blob/2912390662c3c50732add54c94ac9b98d17e1cba/c10/util/Logging.h

from k2.

from k2.

Yes.
I change the first pytorch link, in which pytorch shows how it handles the conflict that could happen WITH GLOG. (TF takes the same way. Thus, it's probably a best practice, as today glog is the somewhat best choice for logging.)

Anyway, let me give a PR first to talk about it.

from k2.

Hi, there is a PR #80 for this issue. Please review it.

To be clear, I want to explain why to add a new logger as an alternative?

• glog is good for logging, which is also the preferred choice for customized "assert".
• use CHECK[_*] instead of assert, b/c assert is not for debugging and error processing. The check function for debugging is better implemented through the logging system. As what glog does.
• glog is either too heavy or not available for some mobile platforms. Thus, there usually is an alternative minimal one that overloads the same common API (macro) of glog. As tf, pytorch do.
• one is chosen from glog or non-glog logger. Here, Option "K2_USE_GLOG" in CMake is for it.

from k2.

I think #80 is somewhat too complicated.

glog is either too heavy or not available for some mobile platforms.

@danpovey Are mobile platforms supported by K2? I don't think they have the ability to train something.

from k2.

Is loguru popular and used by many other projects? I may agree with fangjun here as I feel it's too heavy to involve two log libraries in our project. We can support mobile platforms later if we find we really need to support it (of course we need to enclose third-party log libraries into interfaces with names like K2_, so that we can change the log implementations easily in the future, without changing the calling code).
Anyway, I think writing some code based on glog to support some extra information (e.g. filename, function name, line-number, etc.) would be good enough to our current requirements.

from k2.

from k2.

e.g. filename, function name, line-number, etc.

Do these already in the logging of glog?
Glog doesn't support is output these in the stack trace. But the exact position is not important I think, as the module name is enough to locate the problem(?).

from k2.

from k2.

Emm, the problem just is glog is more than enough, as it introduce too much than needed.
Loguru can act as a minimal glog. Which is also customized implemented by many projects to prevent glog problems(the real heavy thing).

from k2.

from k2.

glog is more than enough, as it introduce too much than needed

I think this is not a problem, we just can choose what we need and use only those functions/Macros. Compared with loguru, I guess glog will be mantained better...

from k2.

Emm, the problem just is glog is more than enough, as it introduce too much than needed.

Is this a problem for K2?

If so, what problems are there of using glog in K2?

How does loguru solve them?

I've never heard of loguru. Could you please list some open source projects using loguru?

from k2.

from k2.

My feeling is (sorry Meixu!) that we can wait till we actually have a problem and at that time do the loguru stuff, but for now just using glog would be fine... with some wrappers to make it K2_CHECK_EQ etc. for future compatibility.
…
On Tue, Aug 4, 2020 at 5:21 PM Fangjun Kuang @.***> wrote: Emm, the problem just is glog is more than enough, as it introduce too much than needed. Is this a problem for K2? If so, what problems are there of using glog in K2? How does loguru solve them? ------------------------------ I've never heard of loguru. Could you please list some open source projects using loguru? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#78 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZFLO42CLIJY335FTFYZP3R67HIZANCNFSM4PPR77MQ .

Guys, I am fine but have to make my point after investigating it. Sorry for my poor expression😅.

Add a K2_* alias define of glog doesn't help. I explain here:
https://github.com/danpovey/k2/pull/80#issuecomment-668564361
https://github.com/danpovey/k2/pull/80#issuecomment-668581277

Emm, I feel it's worthy to face glog thing this earlier. I have encounter glog problems at previous engineering work. More disaster is protobuf, which is itself version incompatible.

from k2.