Skip to content

Conversation

@arng40
Copy link
Contributor

@arng40 arng40 commented Oct 31, 2025

  • Remove all getDataContext() / getWrapperDataContext() / getName() written directly in the error message strings (getDataContext() << “: blabla”).

  • Add missing DataContexts when they are not present in the error DataContext parameters (GEOS_ERROR*, GEOS_ASSERT*, GEOS_WARNING*, GEOS_THROW*)

… link between GEOS_THROW_CTX_IF and LVARRAY_THROW_IF_TEST( EXP, MSG, TYPE )
… in try/catch statements

Problem: Retrieves everything that was thrown, so not just the message.
…y spaces.

The previous condition checked whether an argument was present and whether the option was immediately followed by a value like -test"value", which excluded valid cases like -test "value" et -test     "value".
@arng40 arng40 requested a review from corbett5 as a code owner November 5, 2025 10:56
@arng40 arng40 changed the title Remove getDataContext duplication refactor: Remove getDataContext duplication Nov 5, 2025
@arng40 arng40 added ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: no rebaseline Does not require rebaseline labels Nov 5, 2025
@arng40 arng40 requested a review from herve-gross as a code owner December 3, 2025 15:02
{
GEOS_THROW_IF( (m_xMax[0]<m_xMin[0] || m_xMax[1]<m_xMin[1] || m_xMax[2]<m_xMin[2]),
getCatalogName() << " " << getDataContext() << " "
getCatalogName() << " "
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
getCatalogName() << " "

Copy link
Contributor

Choose a reason for hiding this comment

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

&↓∞

Copy link
Contributor

Choose a reason for hiding this comment

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

There are still more below.

Comment on lines 100 to 101
GEOS_FMT( "{} : the flag `{}` is different from zero, but `{}` is empty, which is inconsistent",
catalogName(), onlyPlotSpecifiedFieldNamesString, fieldNamesString ),
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
GEOS_FMT( "{} : the flag `{}` is different from zero, but `{}` is empty, which is inconsistent",
catalogName(), onlyPlotSpecifiedFieldNamesString, fieldNamesString ),
GEOS_FMT( "The flag `{}` is different from zero, but `{}` is empty, which is inconsistent",
onlyPlotSpecifiedFieldNamesString, fieldNamesString ),

Copy link
Contributor

Choose a reason for hiding this comment

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

&↓∞

Copy link
Contributor

Choose a reason for hiding this comment

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

There are still data contexts in messages below, rather than in the macro parameters

Comment on lines 123 to 124
GEOS_FMT( "{} : the flag `{}` is different from zero, but `{}` is empty, which is inconsistent",
catalogName(), onlyPlotSpecifiedFieldNamesString, fieldNamesString ),
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
GEOS_FMT( "{} : the flag `{}` is different from zero, but `{}` is empty, which is inconsistent",
catalogName(), onlyPlotSpecifiedFieldNamesString, fieldNamesString ),
GEOS_FMT( "the flag `{}` is different from zero, but `{}` is empty, which is inconsistent",
onlyPlotSpecifiedFieldNamesString, fieldNamesString ),

&↓∞

Copy link
Contributor

Choose a reason for hiding this comment

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

There are still data contexts in messages below, rather than in the macro parameters
There are still catalog name too

@MelReyCG
Copy link
Contributor

@arng40 requested a lot of unmeaningful catalog-name / prefixes deletion as:

  • they do not adds up useful info,
  • some of the prefixes should not be shown to the user (internal type names),
  • if we want to generalize this, we should implement that in the DataContext class, to keep track of the input file type.

"Mismatch between the size of " <<
viewKeyStruct::componentNamesString() <<
" and " << viewKeyStruct::componentFractionVsElevationTableNamesString(),
InputError );
Copy link
Contributor

@MelReyCG MelReyCG Dec 30, 2025

Choose a reason for hiding this comment

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

Forgotten data context here?

{
GEOS_THROW_IF( (m_xMax[0]<m_xMin[0] || m_xMax[1]<m_xMin[1] || m_xMax[2]<m_xMin[2]),
getCatalogName() << " " << getDataContext() << " "
getCatalogName() << " "
Copy link
Contributor

Choose a reason for hiding this comment

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

There are still more below.

Comment on lines 100 to 101
GEOS_FMT( "{} : the flag `{}` is different from zero, but `{}` is empty, which is inconsistent",
catalogName(), onlyPlotSpecifiedFieldNamesString, fieldNamesString ),
Copy link
Contributor

Choose a reason for hiding this comment

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

There are still data contexts in messages below, rather than in the macro parameters

Comment on lines 123 to 124
GEOS_FMT( "{} : the flag `{}` is different from zero, but `{}` is empty, which is inconsistent",
catalogName(), onlyPlotSpecifiedFieldNamesString, fieldNamesString ),
Copy link
Contributor

Choose a reason for hiding this comment

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

There are still data contexts in messages below, rather than in the macro parameters
There are still catalog name too

subRegion.getDataContext() <<
": Material " << constitutiveRelation.getDataContext() <<
" does not contain " << viewName,
"Material "<< constitutiveRelation.getName() << "does not contain " << viewName,
Copy link
Contributor

Choose a reason for hiding this comment

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

lacks a space

Comment on lines +356 to +358
ErrorLogger::global().currentErrorMsg()
.addToMsg( errorMsg )
.addContextInfo( getDataContext().getContextInfo().setPriority( 1 ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

Bad merge?

"WellControls " << wellControls.getName() <<
" : Injection stream not specified for well " << subRegion.getName(),
InputError );
"Injection stream not specified for well " << subRegion.getName(),
Copy link
Contributor

Choose a reason for hiding this comment

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

For those 4 messages, should we have "[...] for well sub-region " << subRegion.getName()? To ease readaction, can you give an instance of what is output from one of these messages? (from a real case)

Comment on lines +276 to +277
getWrapperDataContext( viewKeyStruct::injectionStreamString() ),
getWrapperDataContext( viewKeyStruct::injectionTemperatureString() ) );
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 the output will be strange with two data contexts here, what about just one getDataContext()

(m_targetMassRate <= 0.0 && m_targetMassRateTableName.empty()) &&
(m_targetTotalRate <= 0.0 && m_targetTotalRateTableName.empty())),
"WellControls " << getDataContext() << ": You need to specify a phase, mass, or total rate constraint. \n" <<
"You need to specify a phase, mass, or total rate constraint. \n" <<
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this message lacks some separators?

InputError );
GEOS_FMT( "Region `{}` must be a target region of `{}`",
poromechanicsTargetRegionNames[i], this->flowSolver()->getCatalogName() ),
InputError, this->getDataContext(), this->solidMechanicsSolver()->getDataContext() );
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
InputError, this->getDataContext(), this->solidMechanicsSolver()->getDataContext() );
InputError, this->getDataContext(), this->flowSolver()->getDataContext() );

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: 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