Skip to content
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

feat: Adding log parts #3019

Open
wants to merge 258 commits into
base: develop
Choose a base branch
from
Open

feat: Adding log parts #3019

wants to merge 258 commits into from

Conversation

arng40
Copy link
Contributor

@arng40 arng40 commented Mar 4, 2024

This PR aim to improve the readibility of the logs by adding log separation the differents parts of the log and
the first step for displaying specific log or timestep we want.

The log separation identified are

  • Input Initialisation : ProblemManager::postInputInitializationRecursive();
  • Mesh generation : ProblemManager::generateMesh();
  • NumericalMethods: ProblemManager::applyNumericalMethods();
  • Mesh data registration : ProblemManager::egisterDataOnMeshRecursive();
  • Group & subgroups initialization functions : ProblemManager::initialize();
  • Each cycle time-step : EventManager::run()
  • Cleanup : basicCleanup()

Example of an input :

######################################################################
##                             TIMESTEP                             ##
######################################################################
##  - Time       : 00h00m00s out of 1d, 03h46m40s (0% completed)    ##
##                 0 s / 100000 s                                   ##
##  - Delta Time : 01h23m20s (5000 s)                               ##
##  - Cycle      : 0                                                ##
##                                                                  ##

    Attempt:  0, ConfigurationIter:  0, NewtonIter:  0
        ( Rflow ) = ( 4.82e-04 )        ( Rwell ) = ( 4.74e-03 )        ( R ) = ( 4.76e-03 )
        Last LinSolve(iter,res) = (   1, 1.64e-11 )
        compositionalMultiphaseFlow: Max pressure change = 19297.019 Pa (before scaling)
        compositionalMultiphaseFlow: Max component density change = 0.346 kg/m3 (before scaling)
        reservoirSystem: Global solution scaling factor = 1
        compositionalMultiphaseFlow: Max phase volume fraction change = 0.0004
[...]
Rank 0: Writing out restart file at ./staircase_co2_wells_3d_restart_000000004/rank_0000000.hdf5

##                             TIMESTEP                             ##
######################################################################

@arng40 arng40 self-assigned this Mar 4, 2024
src/coreComponents/common/format/table/TableLayout.hpp Outdated Show resolved Hide resolved
src/coreComponents/fileIO/logPart/LogPart.hpp Outdated Show resolved Hide resolved
void begin( std::ostream & os = std::cout );

/**
* @brief Draw the last part of the section. It include the title
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @brief Draw the last part of the section. It include the title
* @brief Draw the last part of the section. It include the title and optionnaly, the end description(s).

* @brief Set the minimal width of a row
* @param minWidth The minimal width of the table
*/
void setMinWidth( integer const & minWidth );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a good feature in the future would be to have a setMaxWidth() with line wrapping, to ensure that a section cannot be too large.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could do this in another PR

src/coreComponents/fileIO/logPart/LogPart.hpp Outdated Show resolved Hide resolved
src/coreComponents/fileIO/logPart/LogPart.cpp Outdated Show resolved Hide resolved
src/coreComponents/fileIO/logPart/LogPart.cpp Outdated Show resolved Hide resolved
src/coreComponents/events/EventManager.cpp Outdated Show resolved Hide resolved
@arng40 arng40 removed ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Nov 6, 2024
@MelReyCG
Copy link
Contributor

MelReyCG commented Nov 7, 2024

Can you share a full log generated after this PR?

Copy link
Contributor

@paveltomin paveltomin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just check what happened with schema/docs changes

@arng40 arng40 requested review from MelReyCG December 10, 2024 10:55
clear();
}

void LogPart::end( std::ostream & os )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please restore the "End of: " output, don't forget to take the full size of the end into account for sizing the width.

Comment on lines 71 to 74
string_view spaces = std::string( maxNameSize - name.size(), ' ' );
string_view nameFormatted = GEOS_FMT( "{}{} ", name, spaces );

m_formattedDescriptions.push_back( GEOS_FMT( "{}{}", nameFormatted, m_descriptionValues[idxName][0] ));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would less mask what you want to do.

Suggested change
string_view spaces = std::string( maxNameSize - name.size(), ' ' );
string_view nameFormatted = GEOS_FMT( "{}{} ", name, spaces );
m_formattedDescriptions.push_back( GEOS_FMT( "{}{}", nameFormatted, m_descriptionValues[idxName][0] ));
string_view spaces = std::string( maxNameSize - name.size(), ' ' );
string_view nameFormatted = GEOS_FMT( "{}{}", name, spaces );
m_formattedDescriptions.push_back( GEOS_FMT( "{} {}", nameFormatted, m_descriptionValues[idxName][0] ));

* @brief Add a description to the logPart by concatening a description name and descriptions values.
* @param name The description name
* @param args Descriptions values to be concatened.
* Descriptions values can be be any types and will be aligned
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Descriptions values can be be any types and will be aligned
* @note Descriptions values can be be any formatted types. Values will be aligned altogether.

Comment on lines +93 to +99
"\n######################################################################\n"
"## TIMESTEP START ##\n"
"######################################################################\n"
"## - Time : 00h08m20s out of 2d, 21h26m40s (0% completed) ##\n"
"## 500 s / 250000 s ##\n"
"## - Delta Time : 00h16m40s (1000 s) ##\n"
"## Description test ##\n\n"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we have the following?

Suggested change
"\n######################################################################\n"
"## TIMESTEP START ##\n"
"######################################################################\n"
"## - Time : 00h08m20s out of 2d, 21h26m40s (0% completed) ##\n"
"## 500 s / 250000 s ##\n"
"## - Delta Time : 00h16m40s (1000 s) ##\n"
"## Description test ##\n\n"
"\n####################################################################\n"
"## TIMESTEP START ##\n"
"####################################################################\n"
"## - Time : 00h08m20s out of 2d, 21h26m40s (0% completed) ##\n"
"## 500 s / 250000 s ##\n"
"## - Delta Time : 00h16m40s (1000 s) ##\n"
"## Description test ##\n\n"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because min width is set to 70

Comment on lines 98 to 112
/// Vector containing formatted description
std::vector< string > m_formattedDescriptions;
/// Name of the description, formatted to be : [Name] : [Values1]\n[Values2]
std::vector< string > m_descriptionNames;
/// Values in the descrpption the description
std::vector< std::vector< string > > m_descriptionValues;

/// title of logPart
string m_logPartTitle;
/// Title footer string
string m_footerTitle;
/// min width of logPart length
size_t m_rowMinWidth = 70;
/// logPart length
size_t m_logPartWidth;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Vector containing formatted description
std::vector< string > m_formattedDescriptions;
/// Name of the description, formatted to be : [Name] : [Values1]\n[Values2]
std::vector< string > m_descriptionNames;
/// Values in the descrpption the description
std::vector< std::vector< string > > m_descriptionValues;
/// title of logPart
string m_logPartTitle;
/// Title footer string
string m_footerTitle;
/// min width of logPart length
size_t m_rowMinWidth = 70;
/// logPart length
size_t m_logPartWidth;
struct Description {
/// Title footer string
string m_title;
/// Name of the description, formatted to be : [Name] : [Values1]\n[Values2]
std::vector< string > m_descriptionNames;
/// Values in the descrpption the description
std::vector< std::vector< string > > m_descriptionValues;
/// Vector containing the descriptions formatted by formatDescriptions()
std::vector< string > m_formattedDescriptionLines;
/// logPart length
size_t m_logPartWidth;
void formatDescriptions();
};
/// min width of logPart length
size_t m_rowMinWidth = 70;
Description startDesc;
Description endDesc;

And so, you should be able to easily restore the addEndDescription() methods, which should be more user friendly from my point of view.

I'm not sure that void formatDescriptions(); should be relocated here (and that its signature is complete), but if the code is well structured, it should be possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants