[Mirror] 16.2 backports

[csiefer2]

Automated mirror of upstream PR trilinos#15138 @trilinos/

Motivation

Backport changes from develop to the 16.2 release branch in preparation for the 16.2 patch release

• Patch stk destructors (Kokkos decorations) trilinos/Trilinos#14881
• Conditioning some ETI directives to both trilinoscouplings and tpetra being ON during configuration trilinos/Trilinos#14946
• Tpetra: Replace scalar comparison with 0.0 to scalar traits zero value in SingletonFilter trilinos/Trilinos#14765
• Commit trilinos@a53d7c8 from Adding missing package dependencies trilinos/Trilinos#15084

[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 Trilinos to version 16.2.0 and introduces several enhancements across packages, including new Tpetra utilities for Stokhos, improved memory management and debugging for Amesos2 solvers, and build system refinements. Feedback highlights the need for proper MPI root-rank guards on new debug print statements in Amesos2 to avoid garbled output. Additionally, improvements are suggested for the efficiency of Tpetra graph construction in Stokhos, specifically regarding row capacity estimation and batching index insertions.

[gemini-code-assist]

This debug block should be guarded by if (this->root_). Since KLU2 is a serial solver, data_.numeric_ will be nullptr on all non-root ranks. Without the guard, every non-root rank will print a failure message to std::cout even if the factorization succeeded on the root rank, leading to confusing and incorrect error logs in parallel environments.

        if(this->root_ && debug_level_ > 0) {
          std::cout << " ** Amesos2::KLU2::numericFactorization failed with status = ";
          if(data_.common_.status == KLU_OK)
            std::cout << "KLU_OK **\n";
          else if (data_.common_.status == KLU_SINGULAR)
            std::cout << "KLU_SINGULAR **\n";
          else if (data_.common_.status == KLU_OUT_OF_MEMORY)
            std::cout << "KLU_OUT_OF_MEMORY **\n";
          else if (data_.common_.status == KLU_INVALID)
            std::cout << "KLU_INVALID **\n";
          else if (data_.common_.status == KLU_TOO_LARGE) 
            std::cout << "KLU_TOO_LARGE **\n";
        }

[gemini-code-assist]

The call to B->description() on line 222 is a no-op because it returns a string that is not used. Additionally, the describe block in the else branch is not guarded by this->root_, which will cause all MPI ranks to print to std::cout simultaneously, leading to garbled output in parallel. This issue also occurs at lines 403-410 and 504-508.

    if (this->root_) {
      if (debug_level_ == 1) {
        std::cout << B->description() << std::endl;
      } else {
        Teuchos::RCP<Teuchos::FancyOStream> fancy = Teuchos::fancyOStream(Teuchos::rcpFromRef(std::cout));
        B->getMap()->describe(*fancy, Teuchos::VERB_EXTREME);
        std::cout << std::endl;
        B->describe(*fancy, Teuchos::VERB_EXTREME);
      }
    }

[gemini-code-assist]

Similar to the KLU2 implementation, B->description() is a no-op and the describe calls should be guarded by if (this->root_) to avoid interleaved output from multiple MPI ranks. This issue also occurs at lines 279-286 and 555-559.

      if (this->root_) {
        if (debug_level_ == 1) {
          std::cout << B->description() << std::endl;
        } else {
          Teuchos::RCP<Teuchos::FancyOStream> fancy = Teuchos::fancyOStream(Teuchos::rcpFromRef(std::cout));
          B->getMap()->describe(*fancy, Teuchos::VERB_EXTREME);
          std::cout << std::endl;
          B->describe(*fancy, Teuchos::VERB_EXTREME);
        }
      }

[gemini-code-assist]

Updating max_num_entry inside the k_it loop is inefficient. It should be updated once per row i after the k_it loop completes. Furthermore, num_entry counts all entries including duplicates across different k values, which can lead to a significant overestimate of the required row capacity in Tpetra::CrsGraph, potentially causing excessive memory allocation.

Suggested change

	for (typename Cijk_type::ik_iterator k_it = Cijk.k_begin(i_it); k_it != Cijk.k_end(i_it); ++k_it) {
	for (typename Cijk_type::ikj_iterator j_it = Cijk.j_begin(k_it); j_it != Cijk.j_end(k_it); ++j_it) {
	++num_entry;
	}
	max_num_entry = (num_entry > max_num_entry) ? num_entry : max_num_entry;
	}
	for (typename Cijk_type::ik_iterator k_it = Cijk.k_begin(i_it); k_it != Cijk.k_end(i_it); ++k_it) {
	for (typename Cijk_type::ikj_iterator j_it = Cijk.j_begin(k_it); j_it != Cijk.j_end(k_it); ++j_it) {
	++num_entry;
	}
	}
	max_num_entry = (num_entry > max_num_entry) ? num_entry : max_num_entry;

[gemini-code-assist]

Inserting global indices one by one in a triple loop is highly inefficient for Tpetra::CrsGraph. It is recommended to accumulate all unique column indices for a given row i and perform a single insertGlobalIndices call per row. This also avoids redundant insertions of the same (i, j) pair for different k values.