illegal choices for linear solvers not intercepted

[Nico Schlömer]

I would like to solve a linear system with CG, and did the following

====================== *snip* ======================
from dolfin import *

# Create mesh and define function space
mesh = UnitSquareMesh(32, 32)
V = FunctionSpace(mesh, "Lagrange", 1)

# Define boundary condition
bc = DirichletBC(V, Constant(0.0), 'on_boundary')

# Define variational problem
u = TrialFunction(V)
v = TestFunction(V)
a = inner(grad(u), grad(v)) * dx
L = Constant(1.0) * v * dx

# Compute solution
u = Function(V)

set_log_level(PROGRESS)

solve(a == L, u, bc,
      solver_parameters={"linear_solver": "cg"})
====================== *snap* ======================

The output of this script is

====================== *snip* ======================
[...]
  Solving linear system of size 1089 x 1089 (PETSc LU solver, umfpack).
====================== *snap* ======================

This is because "cg" not a valid choice for "linear_solver", only "direct" and "iterative" are. If I'm not mistaken, this has changed between 1.0 and 1.1.

[Nico Schlömer (nschloe)]

This little patch here takes care of things:

================ *snip* ================
--- a/dolfin/fem//LinearVariationalSolver.cpp 2013-01-16 16:19:25.447426420 +0100
+++ b/dolfin/fem/LinearVariationalSolver.cpp 2013-01-16 16:27:05.787448401 +0100
@@ -186,13 +186,17 @@
     solver.parameters.update(parameters("krylov_solver"));
     solver.solve(*A, *u->vector(), *b);
   }
- else
+ else if (solver_type == "direct")
   {
     // Solve linear system
     LUSolver solver;
     solver.parameters.update(parameters("lu_solver"));
     solver.solve(*A, *u->vector(), *b);
   }
+ else
+ dolfin_error("LinearVariationalSolver.cpp",
+ "initialize linear solver",
+ "Illegal solver type. Valid choices are \"iterative\" and \"direct\"");

   end();
 }
================ *snap* ================

[Johannes Ring (johannr)]

Shouldn't this be fixed in 1.1.x?

[Johan Hake (johan-hake)]

Yes, and possibly 1.0.x too. But I could only target it to the trunk
series. Not sure why?

Johan

On 01/22/2013 08:08 AM, Johannes Ring wrote:
> Shouldn't this be fixed in 1.1.x?
>

[Johannes Ring (johannr)]

Milestones for 1.1.1 and 1.0.2 were missing. I have added them now.

[Johan Hake (johan-hake)]

It turns out that in revision 7347 Logg changed the behavior in LinearVariationalSolver.cpp so either a KrylovSolver or an LuSolver is instantiated. So it is a actually a regression from 1.0.

The point is that one now cannot pass "cg", or some other previously valid solver type as it only expects "iterative" or "direct". I think the logic should be a bit more elaborated. In 1.0 one can pass any valid iterative or direct solver and the logic in LinearSolver took care of that.

[Anders Logg (logg)]

7347???

revno: 7347
committer: Martin Sandve Alnaes <email address hidden>
branch nick: scratch
timestamp: Mon 2013-01-21 07:42:42 +0100
message:
  Fix test.

--
Anders

On Tue, Jan 22, 2013 at 09:00:13AM -0000, Johan Hake wrote:
> It turns out that in revision 7347 Logg changed the behavior in
> LinearVariationalSolver.cpp so either a KrylovSolver or an LuSolver is
> instantiated. So it is a actually a regression from 1.0.
>
> The point is that one now cannot pass "cg", or some other previously
> valid solver type as it only expects "iterative" or "direct". I think
> the logic should be a bit more elaborated. In 1.0 one can pass any valid
> iterative or direct solver and the logic in LinearSolver took care of
> that.
>
> ** Changed in: dolfin
> Milestone: 1.0.2 => 1.1.1
>

[Johan Hake (johan-hake)]

7165 it should be!

No clue how it became 7347...

Johan

On 01/23/2013 12:03 PM, Anders Logg wrote:
> 7347???
>
> revno: 7347
> committer: Martin Sandve Alnaes <email address hidden>
> branch nick: scratch
> timestamp: Mon 2013-01-21 07:42:42 +0100
> message:
> Fix test.
>
> --
> Anders
>
>
> On Tue, Jan 22, 2013 at 09:00:13AM -0000, Johan Hake wrote:
>> It turns out that in revision 7347 Logg changed the behavior in
>> LinearVariationalSolver.cpp so either a KrylovSolver or an LuSolver is
>> instantiated. So it is a actually a regression from 1.0.
>>
>> The point is that one now cannot pass "cg", or some other previously
>> valid solver type as it only expects "iterative" or "direct". I think
>> the logic should be a bit more elaborated. In 1.0 one can pass any valid
>> iterative or direct solver and the logic in LinearSolver took care of
>> that.
>>
>> ** Changed in: dolfin
>> Milestone: 1.0.2 => 1.1.1
>>
>

[Anders Logg (logg)]

ok, now I see it. It's a regression in a way (cannot pass "cg" or
"gmres") but before this change, the Krylov solver parameters were
ignored completely. I'll try a fix but I assume this will be an
iterative process until we get the logic to work as expected...

--
Anders

On Wed, Jan 23, 2013 at 01:24:50PM +0100, Johan Hake wrote:
> 7165 it should be!
>
> No clue how it became 7347...
>
> Johan
>
> On 01/23/2013 12:03 PM, Anders Logg wrote:
> > 7347???
> >
> > revno: 7347
> > committer: Martin Sandve Alnaes <email address hidden>
> > branch nick: scratch
> > timestamp: Mon 2013-01-21 07:42:42 +0100
> > message:
> > Fix test.
> >
> >> It turns out that in revision 7347 Logg changed the behavior in
> >> LinearVariationalSolver.cpp so either a KrylovSolver or an LuSolver is
> >> instantiated. So it is a actually a regression from 1.0.
> >>
> >> The point is that one now cannot pass "cg", or some other previously
> >> valid solver type as it only expects "iterative" or "direct". I think
> >> the logic should be a bit more elaborated. In 1.0 one can pass any valid
> >> iterative or direct solver and the logic in LinearSolver took care of
> >> that.
> >>
> >> ** Changed in: dolfin
> >> Milestone: 1.0.2 => 1.1.1
> >>
> >
>

[Johannes Ring (johannr)]

Looks like this has been fixed in 1.1.x.

[Johan Hake (johan-hake)]

No it is not fixed.

[Patrick Farrell (pefarrell)]

There is a proposed fix in lp:~pefarrell/dolfin/fix-solver-type ; just waiting to test it.