Skip to content
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

[Core][Logger] Add option to output a time stamp at the beginning of each log line #5676

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

armingeiser
Copy link
Member

@armingeiser armingeiser commented Sep 26, 2019

Similar to the severity level, a flag for adding the time stamp at the beginning of each log line can be set to the Logger.

[time stamp] - [label]: [message]

e.g.

11:26:52 - ShapeOpt: Current value of objective =   1.11393E-07 

This can be activated by:

KM.Logger.GetDefaultOutput().SetTimeStampFlag(True)

@KratosMultiphysics/technical-committee

Copy link
Member

@loumalouomega loumalouomega left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not agree, why not using the timer that already exist in kratos?

Copy link
Member

@loumalouomega loumalouomega left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments

@armingeiser
Copy link
Member Author

I do not agree, why not using the timer that already exist in kratos?
@loumalouomega do you mean putting the function to get the time stamp into the timer.h?

@loumalouomega
Copy link
Member

I do not agree, why not using the timer that already exist in kratos?
@loumalouomega do you mean putting the function to get the time stamp into the timer.h?

I said that before check the code, sorry I thought this was a timer 2.0, nevermind

Copy link
Member

@pooyan-dadvand pooyan-dadvand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the feature and think that it is really useful. However, I have some comments about how it is done:

  1. Please note that the message has a mTime member which I would initialize it in the constructor of the message. So we would get the same timestamp for the same message in all outputs.
  2. I would make the LoggerOutput in charge of timestamp formatting. This make it customizable f like other output formatting and not fixed by the unique message class.
  3. Personally I like more a dense YYYYMMDDHHmmss format which is normally used in log files. Of course, this is my personal opinion and I am open to hear others on it.

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

Successfully merging this pull request may close these issues.

4 participants