festim-dev / FESTIM

Coupled hydrogen/tritium transport and heat transfer modelling using FEniCS
https://festim.readthedocs.io/en/stable/
Apache License 2.0
76 stars 16 forks source link

Logging #731

Open RemDelaporteMathurin opened 3 months ago

RemDelaporteMathurin commented 3 months ago

Sometimes it is hard to know what FESTIM is doing and when it's failing.

We have some print statements here and there but a lot is lost in between.

We would need a more robust way of logging what happens, with different logging levels (just like fenics has a log system).

I believe fenics uses this.

I think we can do something similar.

KulaginVladimir commented 2 weeks ago

@RemDelaporteMathurin Do you want to implement all log_levels of the logger or a reduced variant?

I also think that some re-work would help in the case of parallel computing, As I observe, the current info messages are printed by each core that I provide with mpirun -np.

RemDelaporteMathurin commented 2 weeks ago

What do you think? Would it be better to use the fenics logging system or rather make our own with maybe different levels?

KulaginVladimir commented 2 weeks ago

Adapting the fenics log system seems to be a good idea. We can override some of fenics methods similar to Newton solver.

Though, it's a tough question. I think we can re-create log levels 50, 40, 30, and probably 20, whereas 16, 13, 10 - I haven't tested/used a lot yet.

At least, I think that festim and fenics errors/warnings should be clearly divided, the messages should be appropriately spread across cores during parallel computing, and tracing of festim processes is needed

RemDelaporteMathurin commented 2 weeks ago

Though, it's a tough question. I think we can re-create log levels 50, 40, 30, and probably 20, whereas 16, 13, 10 - I haven't tested/used a lot yet.

We don't have to recreate all the log levels. I think we can start with just a binary system like INFO and NOTHING. And we will figure out later if we need to add levels

At least, I think that festim and fenics errors/warnings should be clearly divided, the messages should be appropriately spread across cores during parallel computing, and tracing of festim processes is needed

By clearly divided do you mean that we shouldn't use fenics' log system?

KulaginVladimir commented 2 weeks ago

By clearly divided do you mean that we shouldn't use fenics' log system?

I meant a dedicated message as:

*** -------------------------------------------------------------------------
*** FESTIM encountered an error. If you are not able to resolve this issue
*** using the information listed below, you can ask for help at
***
***     https://festim.discourse.group/
***
*** Remember to include the error message listed below and, if possible,
*** include a *minimal* running example to reproduce the error.
***
*** -------------------------------------------------------------------------
*** Error:   Error.
*** Reason:  Reason.
*** Where:   Where.
*** Process: Process.
*** -------------------------------------------------------------------------

This also implies that dolfin_error does not suit, I guess

RemDelaporteMathurin commented 2 weeks ago

Yes we don't want to make our own errors like this. But what we want is for users to have enough information about what is going on (eg. Defining boundary conditions, defining source terms, solving nonlinear problem for solubility, etc. ) but with the possibility to disable it if needed.