Add Trilinos for linear solver by yib0liu · Pull Request #69 · polyfem/polysolve

yib0liu · 2024-02-14T23:31:17Z

No description provided.

teseoch

can you try to add to the ci (UNIX only) trilinos installed with apt?

teseoch · 2024-02-15T02:43:46Z

linear-solver-spec.json

        ],
        "doc": "Settings for the AMGCL solver."
    },
+    {


i dont think you need any of these, there are no options for trilinos

you need to add the options from trilinos

teseoch · 2024-02-15T02:44:07Z

src/polysolve/linear/Solver.cpp

+Eigen::MatrixXd test_vertices;
+Eigen::MatrixXd init_vertices;
+std::vector<int> test_boundary_nodes;
+Eigen::MatrixXd remove_boundary_vertices(const Eigen::MatrixXd &vertices, const std::vector<int> &boundary_nodes)


what is this?

teseoch · 2024-02-15T02:44:38Z

src/polysolve/linear/Solver.hpp


 namespace polysolve::linear
 {
+#ifdef POLYSOLVE_LARGE_INDEX


why these changes?

teseoch · 2024-02-15T02:45:26Z

src/polysolve/linear/TrilinosSolver.cpp

+    {
+        if (params.contains("Trilinos"))
+        {
+            if (params["Trilinos"].contains("block_size"))


add these options to the spec

now the block size is a method

teseoch · 2024-02-15T02:46:23Z

src/polysolve/linear/Solver.hpp

+extern Eigen::MatrixXd init_vertices;
+extern Eigen::MatrixXd test_vertices;
+extern std::vector<int> test_boundary_nodes;
+Eigen::MatrixXd remove_boundary_vertices(const Eigen::MatrixXd &vertices, const std::vector<int> &boundary_nodes);


this should be inside trilinos, not here

CMakeLists.txt

codecov · 2024-03-19T05:24:51Z

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 80.84%. Comparing base (0cd21ba) to head (f920ea4).
Report is 17 commits behind head on main.

Files with missing lines	Patch %	Lines
src/polysolve/linear/Solver.hpp	0.00%	1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #69      +/-   ##
==========================================
- Coverage   81.20%   80.84%   -0.37%     
==========================================
  Files          49       47       -2     
  Lines        2001     1879     -122     
  Branches      267      257      -10     
==========================================
- Hits         1625     1519     -106     
+ Misses        376      360      -16

Flag	Coverage Δ
polysolve	`80.84% <0.00%> (-0.37%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

teseoch

we need unit test, with and without block size and null space

teseoch · 2024-03-19T16:18:09Z

CMakeLists.txt

 option(POLYSOLVE_WITH_CUSOLVER      "Enable cuSOLVER library"                           OFF)
 option(POLYSOLVE_WITH_PARDISO       "Enable Pardiso library"                            OFF)
-option(POLYSOLVE_WITH_HYPRE         "Enable hypre"                                       ON)
+option(POLYSOLVE_WITH_HYPRE         "Enable hypre"                                      OFF)


teseoch · 2024-03-19T16:19:26Z

src/polysolve/linear/Solver.hpp


+#include <Eigen/Dense>
+#include <Eigen/Sparse>
+#include <vector>


these are not necessary?

teseoch · 2024-03-19T16:20:21Z

src/polysolve/linear/TrilinosSolver.cpp

+    {
+        if (params.contains("Trilinos"))
+        {
+            if (params["Trilinos"].contains("block_size"))


now the block size is a method

teseoch · 2024-03-19T16:20:32Z

src/polysolve/linear/TrilinosSolver.cpp

+            {
+                conv_tol_ = params["Trilinos"]["tolerance"];
+            }
+            if (params["Trilinos"].contains("is_nullspace"))


this is also a method

is_nullspace is not necessary to exist anymore, so it is removed.

teseoch · 2024-03-19T16:21:29Z

src/polysolve/linear/TrilinosSolver.cpp

+        {
+            if (test_vertices.cols()==3)
+            {
+                reduced_vertices=remove_boundary_vertices(test_vertices,test_boundary_nodes);


this shoudnt be here, the null space is a method

This is removed.

teseoch · 2024-03-19T16:21:58Z

src/polysolve/linear/TrilinosSolver.cpp

+Eigen::MatrixXd test_vertices;
+Eigen::MatrixXd init_vertices;
+std::vector<int> test_boundary_nodes;
+Eigen::MatrixXd remove_boundary_vertices(const Eigen::MatrixXd &vertices, const std::vector<int> &boundary_nodes)


this should be inside an util file

teseoch · 2024-03-19T16:22:19Z

src/polysolve/linear/TrilinosSolver.hpp

+#include <Eigen/Sparse>
+#include <vector>
+
+#ifdef HAVE_MPI


mpi is always here?

teseoch · 2024-03-19T16:22:40Z

src/polysolve/linear/TrilinosSolver.hpp

+// column-major matrix, the solver will actually solve A^T x = b.
+//
+
+extern Eigen::MatrixXd init_vertices;


no extern, this is not part of the api

teseoch · 2024-03-19T16:23:12Z

src/polysolve/linear/TrilinosSolver.hpp

+#include "Teuchos_CommandLineProcessor.hpp"
+#include "Epetra_Map.h"
+#include "Epetra_Vector.h"
+#include "Epetra_CrsMatrix.h"


do we need all these includes?

they are all pointers, we could forward declare

teseoch · 2025-04-15T19:27:55Z

src/polysolve/linear/Solver.hpp

        ///                      and initialized. }
        ///
        virtual void solve(const Ref<const VectorXd> b, Ref<VectorXd> x) = 0;
+        virtual void solve(const Ref<const VectorXd> b, const Ref<const MatrixXd> nullspace, Ref<VectorXd> x) {}


this should call the other solve

teseoch · 2025-04-15T19:29:14Z

src/polysolve/linear/TrilinosSolver.hpp

+
+        // Solve the linear system Ax = b
+        virtual void solve(const Ref<const VectorXd> b, Ref<VectorXd> x) override;
+        virtual void solve(const Ref<const VectorXd> b, const Ref<const MatrixXd> nullspace, Ref<VectorXd> x);


yib0liu changed the title ~~Add Trilinos~~ Add Trilinos for linear solver Feb 14, 2024

teseoch reviewed Feb 15, 2024

View reviewed changes

yib0liu force-pushed the trilinos branch from b29ddcc to e9416ab Compare March 19, 2024 05:16

yib0liu force-pushed the trilinos branch from 25edf97 to 03fc84f Compare March 19, 2024 05:59

teseoch reviewed Mar 19, 2024

View reviewed changes

yib0liu requested a review from teseoch August 12, 2024 21:52

teseoch reviewed Apr 15, 2025

View reviewed changes

teseoch approved these changes Apr 15, 2025

View reviewed changes

Chenyang-Joe force-pushed the trilinos branch from 8641536 to 71271fb Compare January 7, 2026 13:38

Conversation

yib0liu commented Feb 14, 2024

Uh oh!

teseoch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Mar 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

teseoch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Mar 19, 2024 •

edited

Loading