FEniCS / dolfinx

Next generation FEniCS problem solving environment
https://fenicsproject.org
GNU Lesser General Public License v3.0
734 stars 178 forks source link

Change C++ logging library to `spdlog` or `glog` #3195

Closed garth-wells closed 4 months ago

garth-wells commented 4 months ago

Some time ago we adopted loguru for C++ logging. At the time we considered several options (see https://github.com/FEniCS/dolfinx/issues/3). The history is a bit hard to follow, but we did have a PR for using spdlog (https://github.com/FEniCS/dolfinx/pull/311), but eventually adopted loguru (https://github.com/FEniCS/dolfinx/pull/417).

A consideration at the time was a reluctance to introduce a substantial dependency that was not widely packaged. loguru is light and we included the source in the DOFLINx tree.

In the meantime,

Should we switch from loguru to spdlog or glog?

@jhale, @chrisrichardson

chrisrichardson commented 4 months ago

glog is also worth considering. It would be less disruptive as the syntax is the same as what we have in loguru. loguru works well, so I'd like to confirm that the same or better features exist before switching.

jhale commented 4 months ago

I support the change but am also interested in glog.

Both glog and spdlog are available in vcpkg, homebrew, Debian, RedHat, conda (feedstock) and Spack, so no issues there.

glog (unlike spdlog) has a Windows/Visual Studio CI job on Github Actions. It is also a drop in replacement for loguru.

We are not logging heavily in tight loops so performance differences are not hugely important for us.

@chrisrichardson and I have nearly finished the Windows build and have identified loguru as producing strange linking (!) errors when compiling the Python headers. I'm sure this can be fixed, but perhaps the energy is better spent switching quickly to something more modern.

garth-wells commented 4 months ago

I've updated the title to add glog to spdlog.

chrisrichardson commented 4 months ago

https://github.com/FEniCS/dolfinx/tree/chris/glog - seems an easy drop-in.

chrisrichardson commented 4 months ago

On further testing, I think spdlog is a better choice: https://github.com/FEniCS/dolfinx/compare/chris/spdlog

garth-wells commented 4 months ago

Switched to spdlog in #3216