Replace our xout library with Gabi Melman's spdlog#799
Conversation
|
beautiful work Niels! Perhaps you can show us some logger output, comparing old with new. |
|
Thanks @mstaring With this pull request, the logger output should be just the same as with the old logger (but with added thread-safety, and prepared for future extensions, like log level support). But it's a good idea to double-check that all the output is still there! My idea is to further adjust the logging in follow-up pull requests, because this pull quest is already quite large! |
|
great to hear, if you can show me two log files, then we can move ahead as you suggest with follow-up PRs. |
|
The result of the logging is partially online, already 😃 That is: the output to standard I/O is shown by the GitHub CI. I did some comparison between the CI logs at https://github.com/SuperElastix/elastix/actions/workflows/ElastixGitHubActions.yml and they look comparable (comparing the I also compared the "out" directories of a simple registration, after having run both elastix 5.1.0 (release version) and a local build. They look similar (except for changes in the line breaks, from "\r\n" to "\n", the subject of my PR gabime/spdlog#2636), including the "IterationInfo" files. I can show you, of course. |
|
looks great Niels, go ahead! |
|
The pull request now has 14 commits, but I think I'll squash a few of them. I think I would like to just have separate commits for 1. adding the spdlog library, 2. adding the elastix log class, 3. using the new logging, and 4. removing the xout library. |
Also did a checkout of submodules in the CI, and added spdlog copyright and license text to the elastix NOTICE file.
Using Notepad++ v4.8.4 Regular expressions (having enabled ". matches newline"):
Find what: ( [ ]+)xl::xout\["warning"\]([^;]+) << std::endl;
Replace with: $1log::warn\(std::ostringstream{} $2\);
Find what: ( [ ]+)xl::xout\["error"\]([^;]+) << std::endl;
Replace with: $1log::error\(std::ostringstream{} $2\);
Find what: ( [ ]+)elxout([^;]+) << std::endl;
Replace with: $1log::info\(std::ostringstream{} $2\);
Find what: ( [ ]+)xl::xout\["coutonly"\]([^;]+) << std::endl;
Replaced with: $1log::to_stdout\(std::ostringstream{} $2\);
Find what: ( [ ]+)xl::xout\["logonly"\]([^;]+) << std::endl;
Replaced with: $1log::to_log_file\(std::ostringstream{} $2\);
Afterwards, passed string literals directly to the log (without using `std::ostringstream{}`), by:
Find what: std::ostringstream{} << "([^"]+)"\);
Find what: std::ostringstream{}\r\n[ ]+<< "([^"]+)"\);
Replace with: "$1"\);
Replaced `std::ostringstream{}` with `log::get_ostringstream()`.
Replaced `elxout << std::endl;` with `log::info("");`.
Removed `\n` from the "WARNING: The parameter..." error messages that ParameterMapInterface passes to elx::Configuration. Replaced the xout["error"] use case from ReportTerminatingException, in elxMainExeUtilities.cxx
In `Optimizer::PrintSettingsVector`, inserted std::showpoint and std::fixed into a local ostringstream for log::info.
N-Dekker
force-pushed
the
Add-spdlog-as-submodule
branch
from
3f17742 to
f262a29
Compare
Including: pull request SuperElastix/elastix#864 commit SuperElastix/elastix@c3d478e "ENH: Upgrade elastix from C++14 to C++17" pull request SuperElastix/elastix#856 commit SuperElastix/elastix@48c6458 "ENH: Add SetInitialTransformParameterObject to ElastixRegistrationMethod" pull request SuperElastix/elastix#832 commit SuperElastix/elastix@05d2b40 ENH: Support "ShowProgressPercentage" parameter (`false` by default) pull request SuperElastix/elastix#815 commit SuperElastix/elastix@c4ef707 "ENH: Add `ElastixLogLevel` to the ITK interface" pull request SuperElastix/elastix#799 "Replace our xout library with Gabi Melman's spdlog" commit SuperElastix/elastix@085a393 "ENH: Add spdlog (v1.11.0) to ThirdParty subdirectory as git submodule"
Including: pull request SuperElastix/elastix#864 commit SuperElastix/elastix@c3d478e "ENH: Upgrade elastix from C++14 to C++17" pull request SuperElastix/elastix#856 commit SuperElastix/elastix@48c6458 "ENH: Add SetInitialTransformParameterObject to ElastixRegistrationMethod" pull request SuperElastix/elastix#832 commit SuperElastix/elastix@05d2b40 ENH: Support "ShowProgressPercentage" parameter (`false` by default) pull request SuperElastix/elastix#815 commit SuperElastix/elastix@c4ef707 "ENH: Add `ElastixLogLevel` to the ITK interface" pull request SuperElastix/elastix#799 "Replace our xout library with Gabi Melman's spdlog" commit SuperElastix/elastix@085a393 "ENH: Add spdlog (v1.11.0) to ThirdParty subdirectory as git submodule"
2 participants
spdlog is especially interesting to elastix because it offers thread support, it allows logging level selection (to be used later), and it is actively maintained.