Adding logLevel documentation #3089

arng40 · 2024-04-23T08:52:43Z

This PR aim to document all logLevel in GEOS and to invite the contributors to set descriptions when they enable the logLevel wrapper:

Currently, we have this output in all Group documentation files:

========= ========= ======== ================
Name      Type      Default  Description                                                                             
========= ========= ======== ================
[...]
logLevel  integer   0        Log level     
========= ========= ======== ================

This PR brings more context to all logLevels of GEOS:

I.e. SolidMechanics_LagrangianFEM.rst :

========= ========= ======== ========================================================================================
Name      Type      Default  Description                                                                             
========= ========= ======== ========================================================================================
[...]
logLevel  integer   0        | Sets the level of information to write in the standard output (the console typically).
                             | A level of 0 outputs minimal information, higher levels require more.                 
                             | logLevel >= 1                                                                         
                             |  - Information on line search                                                         
                             |  - Information on global solution scaling factor                                      
                             |  - Information on the timestep                                                        
                             |  - Print residual norm                                                                
                             | logLevel >= 1 and configuration didn't converge                                       
                             |  - Information about testing new configuration and print the time step                
                             | logLevel >= 1 and incorrect solution                                                  
                             |  - Information about line search failed                                               
                             | logLevel >= 1 and linear system                                                       
                             |  - Information on number of iterations and residual reduction                         
                             | logLevel >= 1 and non linear system                                                   
                             |  - Information on each newton Iteration                                               
                             | logLevel >= 1 and residual norm above the max allowed residual norm                   
                             |  - Indicate allowed residual norm                                                     
                             | logLevel >= 2                                                                         
                             |  - Output to screen the assembled linear system and solutions (matrices and vectors)  
                             |  - Infos on residuals values                                                          
                             | logLevel >= 2 and target set is empty                                                 
                             |  - Warning about boundary conditions                                                  
                             | logLevel >= 3                                                                         
                             |  - Output to file the assembled linear system and solutions (matrices and vectors)    
========= ========= ======== ========================================================================================

MelReyCG · 2024-05-21T08:36:59Z

src/coreComponents/constitutive/relativePermeability/RelpermDriver.cpp

+ appendLogLevel( { "logLevel >= 1", "Infos about RelpermDriver" } );
+ appendLogLevel( { "logLevel >= 1", "Indicate if the internal results are consistent" } );


I would have expect either this syntax:

appendLogLevel( { "logLevel >= 1", "Infos about RelpermDriver" }, { "logLevel >= 1", "Indicate if the internal results are consistent" } );

Or this one

appendLogLevel( "logLevel >= 1", "Infos about RelpermDriver" ); appendLogLevel( "logLevel >= 1", "Indicate if the internal results are consistent" );

The second one is more convenient

MelReyCG · 2024-05-21T08:39:14Z

src/coreComponents/dataRepository/Group.cpp

+ try
+ {
+ WrapperBase & wrapper = getWrapperBase( logLevelName );
+ appendLogLevelToWrapper( wrapper, log );
+ }catch( std::domain_error const & )
+ {
+ WrapperBase & wrapper = registerWrapper( logLevelName, &m_logLevel );
+ enableLogLevelInput();
+ appendLogLevelToWrapper( wrapper, log );
+ }


In this project, try-catch is for adding more info to errors, not to recover from an erroneous state.
Use a if-else syntax.

MelReyCG · 2024-05-21T08:57:41Z

src/coreComponents/dataRepository/Group.hpp

+ * @brief Append a log level/description to the description of the wrapped object
+ * @param log An entry (log level/ log Description) to append
+ */
+ void appendLogLevel( std::pair< const std::string, const std::string > log );


appendLogLevelDescription( string_view loggingCondition, string_view caseDescription ) ?

MelReyCG · 2024-05-21T08:59:36Z

src/coreComponents/dataRepository/WrapperBase.hpp

+ /// Map for building the log level string for each wrapper
+ /// key : logLevel, values : description(s) for a level
+ std::map< std::string, std::vector< std::string > > m_logLevelsDescriptions;


Only one logLevel per Group = only one map per Group
Every wrapper should not have logLevel functionnalities, Group should.
Those method & attributes you added in Wrapper are not at the right place.

MelReyCG

Blocking comments

…level-doc

codecov · 2024-06-20T14:00:20Z

Codecov Report

Attention: Patch coverage is 79.50311% with 33 lines in your changes missing coverage. Please review.

Project coverage is 55.75%. Comparing base (3bf12d2) to head (b47ffe8).
Report is 8 commits behind head on develop.

❗ Current head b47ffe8 differs from pull request most recent head c56f007

Please upload reports for the commit c56f007 to get more accurate results.

Files	Patch %	Lines
...sicsSolvers/surfaceGeneration/SurfaceGenerator.cpp	0.00%	6 Missing ⚠️
...hysicsSolvers/multiphysics/HydrofractureSolver.cpp	0.00%	4 Missing ⚠️
...s/physicsSolvers/simplePDE/PhaseFieldDamageFEM.cpp	0.00%	3 Missing ⚠️
...onstitutive/relativePermeability/RelpermDriver.cpp	0.00%	2 Missing ⚠️
...olvers/solidMechanics/SolidMechanicsStateReset.cpp	0.00%	2 Missing ⚠️
...ers/surfaceGeneration/EmbeddedSurfaceGenerator.cpp	0.00%	2 Missing ⚠️
src/coreComponents/fileIO/Outputs/ChomboIO.cpp	0.00%	1 Missing ⚠️
src/coreComponents/fileIO/Outputs/VTKOutput.cpp	0.00%	1 Missing ⚠️
...onents/linearAlgebra/interfaces/hypre/HypreMGR.cpp	0.00%	1 Missing ⚠️
...mponents/mesh/simpleGeometricObjects/Rectangle.cpp	0.00%	1 Missing ⚠️
... and 10 more

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3089      +/-   ##
===========================================
+ Coverage    55.71%   55.75%   +0.03%     
===========================================
  Files         1031     1031              
  Lines        87698    87823     +125     
===========================================
+ Hits         48863    48962      +99     
- Misses       38835    38861      +26

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

paveltomin · 2024-06-20T14:32:00Z

How is all that connects with proposal about logLevel labels in #3014 ?

arng40 · 2024-06-20T15:03:06Z

This PR is related to the #3179 and not the #3014.

TotoGaz

Could we discuss the this in a meeting first?

TotoGaz · 2024-06-20T21:53:59Z

src/coreComponents/dataRepository/Group.hpp

+ * @param levelCondition The level condition to append
+ * @param logDescription The log description to append
+ */
+ void appendLogLevelDescription( string_view levelCondition, string_view logDescription );


Is there a way, instead of having now 4 public member functions and 2 attributes directly related to logs (and a entry in the viewKeysStruct) to gather all the behaviors related to the log into a dedicated class?
I personally do not want to add more and more stuff into Group which is already big enough (+/- 120 public member functions IIRC).

A bit off-topic comment : I don't know if it would help, but in a long-term effort. I would say that if we use a similar approach with the group's wrappers with a WrapperContainers, we could remove around 36 members (including const/non-const), allowing to clarify that methods like "size()" are related to Wrappers.

arng40 · 2024-06-24T09:37:12Z

@TotoGaz Thanks for your feedback, in reply to your suggestion :
Would a highly coupled class be an issue? Group would only have 1 attribute & 1 getter for this class.
Or is it better to have a purely not coupled class? Group would have 1 attribute, 1 getter, and 1 method (appendLogLevelDescription()).

Add a way to expend the description of a wrapper

fbe6085

MelReyCG changed the title ~~Add a way to expend the description of a wrapper~~ Adding logLevel documentation Apr 24, 2024

arng40 added 4 commits April 25, 2024 14:33

loglevel first iteration

de6c78e

log level documentation sorted

55ef101

log-level doc correction

b55bb04

enable log level + small corretion appendLogLevel

d333678

MelReyCG requested changes May 21, 2024

View reviewed changes

melvin review ( archi updated )

12da842

MelReyCG assigned arng40 May 21, 2024

arng40 added 9 commits May 21, 2024 17:18

renaming / constness / refacto

65d8ae9

uncrustify

f957cea

Merge remote-tracking branch 'origin/develop' into feature/dudes/log-…

69f94df

…level-doc

add missing log

8de5703

update comment

a720b27

adding new doc + update

dcf4fa7

update solver log

78badfa

log message correction

0c32b52

doc corrections

ed7a285

arng40 marked this pull request as ready for review May 27, 2024 07:42

arng40 requested review from ryar9534 and MelReyCG May 27, 2024 07:45

arng40 added 3 commits June 19, 2024 17:06

rst added

c3cdc0d

Merge remote-tracking branch 'origin/develop' into feature/dudes/log-…

de56ea7

…level-doc

fix and updates rst

b47ffe8

arng40 requested review from sframba, CusiniM, paveltomin and dkachuma June 20, 2024 13:14

paveltomin closed this Jun 20, 2024

paveltomin reopened this Jun 20, 2024

arng40 linked an issue Jun 20, 2024 that may be closed by this pull request

Review all log level Wrapper documentation (Wrapper::appendDescription()), #3179

Open

TotoGaz reviewed Jun 20, 2024

View reviewed changes

small update

c56f007

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding logLevel documentation #3089

Adding logLevel documentation #3089

arng40 commented Apr 23, 2024 •

edited

Loading

MelReyCG May 21, 2024

arng40 May 21, 2024

MelReyCG May 21, 2024

MelReyCG May 21, 2024

MelReyCG May 21, 2024

MelReyCG left a comment

codecov bot commented Jun 20, 2024 •

edited

Loading

paveltomin commented Jun 20, 2024

arng40 commented Jun 20, 2024 •

edited

Loading

TotoGaz left a comment

TotoGaz Jun 20, 2024 •

edited

Loading

MelReyCG Jun 24, 2024

arng40 commented Jun 24, 2024 •

edited

Loading

		appendLogLevel( { "logLevel >= 1", "Infos about RelpermDriver" } );
		appendLogLevel( { "logLevel >= 1", "Indicate if the internal results are consistent" } );

Adding logLevel documentation #3089

Are you sure you want to change the base?

Adding logLevel documentation #3089

Conversation

arng40 commented Apr 23, 2024 • edited Loading

MelReyCG May 21, 2024

Choose a reason for hiding this comment

arng40 May 21, 2024

Choose a reason for hiding this comment

MelReyCG May 21, 2024

Choose a reason for hiding this comment

MelReyCG May 21, 2024

Choose a reason for hiding this comment

MelReyCG May 21, 2024

Choose a reason for hiding this comment

MelReyCG left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 20, 2024 • edited Loading

Codecov Report

paveltomin commented Jun 20, 2024

arng40 commented Jun 20, 2024 • edited Loading

TotoGaz left a comment

Choose a reason for hiding this comment

TotoGaz Jun 20, 2024 • edited Loading

Choose a reason for hiding this comment

MelReyCG Jun 24, 2024

Choose a reason for hiding this comment

arng40 commented Jun 24, 2024 • edited Loading

arng40 commented Apr 23, 2024 •

edited

Loading

codecov bot commented Jun 20, 2024 •

edited

Loading

arng40 commented Jun 20, 2024 •

edited

Loading

TotoGaz Jun 20, 2024 •

edited

Loading

arng40 commented Jun 24, 2024 •

edited

Loading