-
Notifications
You must be signed in to change notification settings - Fork 841
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
Adding logger to SU2 #1991
Labels
Comments
This is a nice-to-have but there are 2600 uses of cout << in SU2, are you sure you want to refactor all of them? |
There are some tricks possible to add the cout << "Values at node<< nodeId << " are " << val[0] << " " << val[1] << " " << val[3] << endl; can be transformed directly to LOG("Values at node {} are {}",nodeId,val); Equivalently LOG << fmt::format("Values at node {} are {}",nodeId,val); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There is a lot of boilerplate code in the code base for screen output like
This is not a good example from many points of view.
From performance-wise overly used
std::endl
is a killer. If one needs to redirect output to file it is additional loss etc.From the user's perspective, it is not possible to set the logging level. At least a three-level logging would be nice (INFO/WARNING/DEBUG) this also makes life easier for developers too.
It is also nice to have a rotating log file if one runs longer cases on the HPC systems. After a while the log files getting so bloating
Describe the solution you'd like
A configurable logger would be better with defaults not changing the current system
In the code
For general messages
There is also a lot of
--------------- Start Solver ----------
type headers in the code we can automate this asHEADER << "Start Solver";
Describe alternatives you've considered
A proper choice of logging library is required. Alternatives I considered:
AixLog
<<
operatorspdlog
easylogging
<<
operatorglog
<<
operatorI am in favor of spdlog library
Additional context
Add any other context or screenshots about the feature request here.
The text was updated successfully, but these errors were encountered: