Skip to content

feat: Add command-line option to control developper information output in diagnostic messages#3980

Open
MelReyCG wants to merge 16 commits into
developfrom
feature/rey/diagnose-level
Open

feat: Add command-line option to control developper information output in diagnostic messages#3980
MelReyCG wants to merge 16 commits into
developfrom
feature/rey/diagnose-level

Conversation

@MelReyCG
Copy link
Copy Markdown
Contributor

@MelReyCG MelReyCG commented Feb 23, 2026

This PR propose to introduce a new command-line option to control diagnostic log verbosity in GEOS. The goal is to reduce the verbosity of logs by default while still providing detailed developer information when needed.

New option:

-d, --diagnostic-info-level,  Diagnostic message information level:
                                0 = (default) basic information,
                                1 = developper information (source file, stack-trace) for errors only,
                                2 = developper information for warnings & errors

For an error, -d 0 would give:

***** ExternalError
***** Rank 4
***** Message from Signal (detected from Signal Handler):
Floating point error encountered:
Unknown reason.

... and -d 1 would give:

***** ExternalError
***** Rank 4
***** Message from Signal (detected from Signal Handler):
Floating point error encountered:
Unknown reason.

***** StackTrace of 15 frames
  - Frame  0:  /lib64/libc.so.6 
  - Frame  1:  void geos::isothermalCompositionalMultiphaseBaseKernels::internal::kernelLaunchSelectorCompSwitch<int, geos::isothermalCompositionalMultiphaseBaseKernels::GlobalComponentFractionKernelFactory::createAndLaunch<RAJA::policy::sequential::seq_exec>(int, geos::ObjectManagerBase&)::{lambda(auto:1)#1}>(int, geos::isothermalCompositionalMultiphaseBaseKernels::GlobalComponentFractionKernelFactory::createAndLaunch<RAJA::policy::sequential::seq_exec>(int, geos::ObjectManagerBase&)::{lambda(auto:1)#1}&&) 
  - Frame  2:  geos::CompositionalMultiphaseWell::updateGlobalComponentFraction(geos::WellElementSubRegion&) const 
[...]
  - Frame 11:  geos::GeosxState::run() 
  - Frame 12:  main 
  - Frame 13:  __libc_start_main 
  - Frame 14:  _start 
*****

The last option, -d 2, also enables the output of stack trace and source file for warnings.

Stack traces and source files are still included in the YAML output through the -e option, but now users can control the presence of developper information in the log through this new -d option.

Comment thread src/coreComponents/common/logger/ErrorHandling.hpp Outdated
Comment thread src/coreComponents/common/logger/ErrorHandling.hpp Outdated
Comment thread src/coreComponents/common/logger/ErrorHandling.hpp
Comment thread src/coreComponents/common/logger/GeosExceptions.cpp Outdated
@MelReyCG MelReyCG changed the title Add command-line option to control developper information output in diagnostic messages feat: Add command-line option to control developper information output in diagnostic messages Feb 23, 2026
@arng40 arng40 self-requested a review February 26, 2026 15:54
@MelReyCG MelReyCG added ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Feb 27, 2026
./bin/geosx --help

This should print out a brief summary of the available command line arguments:
This should print out the available command line arguments with their description:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
This should print out the available command line arguments with their description:
This should print out the available command line arguments with their descriptions:

break;
case DIAGNOSTIC_INFO_LEVEL:
{
DiagnosticInfoLevel infoLevel = DiagnosticInfoLevel( std::stoi( opt.arg ) );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure how these work but do we need to check in case the user gives something like -d -3?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just seen how it's implemented. Ignore this comment.


/**
* @brief Set the diagnostic messages information level. The higher, the more verbose and
* developper-oriented the messages will be.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
* developper-oriented the messages will be.
* developer-oriented the messages will be.

/// Avoid concurrent access between threads for yaml outputs
std::mutex m_errorHandlerYamlMutex;
/// Indicated if the source file information output is enabled for a given message type
stdArray< bool, size_t( MsgType::Count ) > m_msgTypeSourceInfoEnabled;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(Might not be a suggestion/review but more like a question)
Here m_msgTypeSourceInfoEnabled is correctly initialized in the setDiagnosticInfoLevel( DiagnosticInfoLevel::Basic ) call in the ErrorLogger() constructor.
But if someone modifies the code and for some reason setDiagnosticInfoLevel( ... ) is called too late, or not called at all (through another constructor? or only in an if condition? or something else?) m_msgTypeSourceInfoEnabled will be left uninitialized and produce UB (maybe using garbage booleans producing unexpected results).

Value initialization with empty braces (where bools are set to false by default when value initializing) could prevent UB, and always produce the same result if this theoretical unhappy mentioned earlier path is run.

Suggested change
stdArray< bool, size_t( MsgType::Count ) > m_msgTypeSourceInfoEnabled;
stdArray< bool, size_t( MsgType::Count ) > m_msgTypeSourceInfoEnabled {};

I've still not come across value initialization in GEOS so maybe there is a reason for it?

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

Labels

ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants