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

refactor: Refactoring dump input option #3566

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

Conversation

arng40
Copy link
Contributor

@arng40 arng40 commented Feb 24, 2025

This PR aim to refactor, with the tableFormatter, the dumping output fonction.
When we have unused input option to an XML element node, we get this kind of input :

***** LOCATION: /data/pau901/VDG_GSI/ARNAUDD/geosRepos/codes/GEOS/src/coreComponents/dataRepository/Group.cpp:247
***** Controlling expression (should be false): processedAttributes.count( attributeName ) == 0
***** Rank 0: Error in compflow (2ph_cap_1d_ihu.xml, l.5): XML Node at '/Problem/Solvers/CompositionalMultiphaseFVM' contains unused attribute 'targeftRegions'.
Valid attributes are:
  |         name         |  opt/req  | Description 
  |----------------------|-----------|-----------------------------------------
  | cflFactor            | OPTIONAL  | Factor to apply to the `CFL condition <http://en.wikipedia.org/wiki/Courant-Friedrichs-Lewy_condition>`_ when calculating the maximum allowable time step. Values should be in the interval (0,1]  
  | discretization       | REQUIRED  | Name of discretization object (defined in the :ref:`NumericalMethodsManager`) to use for this solver. For instance, if this is a Finite Element Solver, the name of a :ref:`FiniteElement` should be specified. If this is a Finite Volume Method, the name of a :ref:`FiniteVolume` discretization should be specified. 
  | targetRegions        | REQUIRED  | Allowable regions that the solver may be applied to. Note that this does not indicate that the solver will be applied to these regions, only that allocation will occur such that the solver may be applied to these regions. The decision about what regions this solver will beapplied to rests in the EventManager. 
  | initialDt            | OPTIONAL  | Initial time-step value required by the solver to the event manager. 
  | writeLinearSystem    | OPTIONAL  | Write matrix, rhs, solution to screen ( = 1) or file ( = 2). 
  | logLevel             | OPTIONAL  | Sets the level of information to write in the standard output (the console typically).
Information output from lower logLevels is added with the desired log level 
  | isThermal            | OPTIONAL  | Flag indicating whether the problem is thermal or not. 
  | allowNegativePressure | OPTIONAL  | Flag indicating if negative pressure is allowed 
  | maxAbsolutePressureChange | OPTIONAL  | Maximum (absolute) pressure change in a Newton iteration 
  | maxSequentialPressureChange | OPTIONAL  | Maximum (absolute) pressure change in a sequential iteration, used for outer loop convergence check 

With the table, we get instead :

-------------------------------------------------------------------------------------------------------------------------------------
|               name               |    flag    |                                    Description                                    |
-------------------------------------------------------------------------------------------------------------------------------------
|  logLevel                        |  OPTIONAL  |  Log level                                                                        |
|  lineSearchAction                |  OPTIONAL  |  How the line search is to be used. Options are:                                  |
|                                  |            |  * None    - Do not use line search.                                              |
|                                  |            |  * Attempt - Use line search. Allow exit from line search without achieving       |
|                                  |            |  smaller residual than starting residual.                                         |
|                                  |            |  * Require - Use line search. If smaller residual than starting resdual is not    |
|                                  |            |  achieved, cut time-step.                                                         |
|  lineSearchInterpolationType     |  OPTIONAL  |  Strategy to cut the solution update during the line search. Options are:         |
|                                  |            |  * Linear                                                                         |
|                                  |            |  * Parabolic                                                                      |
|  lineSearchMaxCuts               |  OPTIONAL  |  Maximum number of line search cuts.                                              |
|  lineSearchCutFactor             |  OPTIONAL  |  Line search cut factor. For instance, a value of 0.5 will result in the          |
|                                  |            |  effective application of the last solution by a factor of (0.5, 0.25, 0.125,     |
|                                  |            |  ...)                                                                             |
|  lineSearchStartingIteration     |  OPTIONAL  |  Iteration when line search starts.                                               |
|  lineSearchResidualFactor        |  OPTIONAL  |  Factor to determine residual increase (recommended values: 1.1 (conservative),   |
|                                  |            |  2.0 (relaxed), 10.0 (aggressive)).                                               |
|  normType                        |  OPTIONAL  |  Norm used by the flow solver to check nonlinear convergence. Valid options:      |
|                                  |            |  * Linfinity                                                                      |
|                                  |            |  * L2                                                                             |
|  minNormalizer                   |  OPTIONAL  |  Value used to make sure that residual normalizers are not too small when         |
|                                  |            |  computing residual norm.                                                         |
|  newtonTol                       |  OPTIONAL  |  The required tolerance in order to exit the Newton iteration loop.               |
|  newtonMaxIter                   |  OPTIONAL  |  Maximum number of iterations that are allowed in a Newton loop.                  |
|  newtonMinIter                   |  OPTIONAL  |  Minimum number of iterations that are required before exiting the Newton loop.   |
|  maxAllowedResidualNorm          |  OPTIONAL  |  Maximum value of residual norm that is allowed in a Newton loop                  |
|  allowNonConverged               |  OPTIONAL  |  Allow non-converged solution to be accepted. (i.e. exit from the Newton loop     |
|                                  |            |  without achieving the desired tolerance)                                         |
|  timeStepDecreaseIterLimit       |  OPTIONAL  |  Fraction of the max Newton iterations above which the solver asks for the        |
|                                  |            |  time-step to be decreased for the next time-step.                                |
|  timeStepIncreaseIterLimit       |  OPTIONAL  |  Fraction of the max Newton iterations below which the solver asks for the        |
|                                  |            |  time-step to be increased for the next time-step.                                |
|  timeStepDecreaseFactor          |  OPTIONAL  |  Factor by which the time-step is decreased when the number of Newton iterations  |
|                                  |            |  is large.                                                                        |
|  timeStepIncreaseFactor          |  OPTIONAL  |  Factor by which the time-step is increased when the number of Newton iterations  |
|                                  |            |  is small.                                                                        |
|  subcycling                      |  OPTIONAL  |  Flag to decide whether to iterate between sequentially coupled solvers or not.   |
|  nonlinearAccelerationType       |  OPTIONAL  |  Nonlinear acceleration type for sequential solver.                               |
-------------------------------------------------------------------------------------------------------------------------------------

@arng40 arng40 changed the title ci: fix 1d edfm case and add it to ats (#3546) refactor: Refactoring dump input option Feb 24, 2025
@arng40 arng40 self-assigned this Feb 24, 2025
@arng40 arng40 added type: cleanup / refactor Non-functional change (NFC) flag: no rebaseline Does not require rebaseline labels Feb 24, 2025
@arng40 arng40 marked this pull request as ready for review February 25, 2025 13:30
@@ -241,6 +241,68 @@ void TableTextFormatter::setLinks( std::vector< TableLayout::Column > & columns
}
}

std::string_view ltrim( std::string_view s )
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be constexpr.

return {};
}

bool formatLinesWithMaxSize( std::vector< std::string > & cellLines, size_t maxLength )
Copy link
Contributor

Choose a reason for hiding this comment

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

those two functions can be helpful! You could generalize a bit (erase table/cell namings) and add them to StringUtilities.xpp :)

Also, this function produces incorrect results for { "Les shorts c'est pratique, ça laisse les jambes bien au frais.", 8} (obviously, also for maxLength=6).

Comment on lines 579 to 582
// Contain the max width for each column
size_t m_tableColumnMaxWidth = std::numeric_limits< size_t >::max();
// Indicate if a max column width has been set
bool m_hasMaxColumnWidthSet = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

you could only keep one attribute here:

using NoColumnMaxWidth = std::numeric_limits< size_t >::max();
size_t m_tableColumnMaxWidth = NoColumnMaxWidth;

And in isMaxColumnWidthSet() : return m_tableColumnMaxWidth == NoColumnMaxWidth;

@@ -302,13 +305,29 @@ string Group::dumpInputOptions() const
{
string rval;

bool writeHeader = true;
TableLayout logLayout( "", {TableLayout::Column()
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
TableLayout logLayout( "", {TableLayout::Column()
TableLayout const logLayout( "", {TableLayout::Column()

@@ -727,11 +727,11 @@ class Wrapper final : public WrapperBase
targetNode,
inputFlag == InputFlags::REQUIRED );
GEOS_THROW_IF( !m_successfulReadFromInput,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you show a comprehensive output of the error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

***** Input value: ''
***** LOCATION: /data/pau901/VDG_GSI/ARNAUDD/geosRepos/codes/GEOS4/src/coreComponents/dataRepository/Wrapper.hpp:729
***** Controlling expression (should be false): !m_successfulReadFromInput
***** Rank 3: XML Node CompositionalMultiphaseFVM (2ph_cap_1d_ihu.xml, l.5) with name=compflow is missing required attribute 'discretization'.
For more details, please refer to documentation at:
http://geosx-geosx.readthedocs-hosted.com/en/latest/docs/sphinx/userGuide/Index.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flag: no rebaseline Does not require rebaseline type: cleanup / refactor Non-functional change (NFC)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants