[Mirror] MueLu: Correct TekoSmoother::Apply when InitialGuessIsZero is false

[csiefer2]

Automated mirror of upstream PR trilinos#15173 @trilinos/muelu

[github-actions]

CDash for AT1 results [Only accessible from Sandia networks]
CDash for AT2 results [Currently only accessible from Sandia networks]

[gemini-code-assist]

Code Review

This pull request updates the TekoSmoother::Apply method to correctly handle the InitialGuessIsZero flag by implementing separate logic for zero and non-zero initial guesses. It also adds a comprehensive unit test for the TekoSmoother. Feedback includes moving a null check for rgMapExtractor to cover all code paths, removing a redundant vector copy when the initial guess is zero, and cleaning up an unused variable in the residual-based application path.

[gemini-code-assist]

The null check for rgMapExtractor should be moved outside the if (InitialGuessIsZero) block. It is required for both branches, as rgMapExtractor is used at line 292 in the else branch to update the Thyra residual vector.

Suggested change

	auto rgMapExtractor = bA_->getRangeMapExtractor();
	Teuchos::RCP<const MapExtractor> rgMapExtractor = bA_->getRangeMapExtractor();
	TEUCHOS_TEST_FOR_EXCEPT(Teuchos::is_null(rgMapExtractor));
	if (InitialGuessIsZero) {
	TEUCHOS_TEST_FOR_EXCEPT(Teuchos::is_null(rgMapExtractor));
	auto rgMapExtractor = bA_->getRangeMapExtractor();
	TEUCHOS_TEST_FOR_EXCEPT(Teuchos::is_null(rgMapExtractor));
	if (InitialGuessIsZero) {

[gemini-code-assist]

When InitialGuessIsZero is true, copying the initial guess X to thyProdX is redundant. The inverseOp_->apply call on line 270 uses beta = 0.0, which means the initial contents of the output vector thyX are ignored and overwritten. Removing this copy and the associated cast avoids unnecessary work.

[gemini-code-assist]

The variable thyProdX and its associated null check are unused in the else branch. They should be removed to improve code clarity and avoid unnecessary casting.

[cgcgcg]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the Apply method in MueLu_TekoSmoother_decl.hpp to support non-zero initial guesses and improves code maintainability by introducing local lambdas for Thyra vector operations. Additionally, it adds unit tests for the Teko smoother and updates template syntax for C++11 compatibility. Review feedback suggests qualifying the MueLu::Utilities namespace to prevent compilation errors, adjusting the type of alphaX for consistency with the MultiVector update signature, and removing a redundant vector copy when the initial guess is zero.

[gemini-code-assist]

The Utilities class is likely not defined in this scope without a full namespace qualification or a local alias. Given that other utility classes in this file (like Xpetra::ThyraUtils) are fully qualified, you should use the fully qualified name for MueLu::Utilities to ensure successful compilation.

Suggested change

	auto residual = Utilities::Residual(*A_, X, B);
	auto residual = MueLu::Utilities<Scalar, LocalOrdinal, GlobalOrdinal, Node>::Residual(*A_, X, B);

[gemini-code-assist]

The parameter alphaX should be of type Scalar to be consistent with the Xpetra::MultiVector::update signature and the values passed to the lambda (STS::zero() and STS::one()).

Suggested change

	const typename STS::magnitudeType alphaX) {
	const Scalar alphaX) {

[gemini-code-assist]

When InitialGuessIsZero is true, initializing the Thyra vector from X is unnecessary. Since inverseOp_->apply is called with beta=0.0 (line 285), the initial content of the output vector is ignored. Setting initializeFromX to false avoids a redundant copy of X (which might contain garbage or NaNs).

Suggested change

	solveWithThyra(Teuchos::rcpFromRef(B), /*initializeFromX=*/true, STS::zero());
	solveWithThyra(Teuchos::rcpFromRef(B), /*initializeFromX=*/false, STS::zero());