On Fri, 6 Feb 2009, Derek Gaston wrote:
> On Feb 6, 2009, at 3:47 PM, Roy Stogner wrote:
>> Supporting old APIs is always good. If you want to stick a deprecated()
>> warning in there now and plan to get rid of it in the future, though,
>> that's fine.
>> In the near term, I'd like to have a factory method that takes a
>> PreconditionerType and generates a Preconditioner.
> Hmmm... so I think you're thinking of a slightly different (but similar)
> model to what I've implemented.
Sort of; what you've implemented is a subset of what I'm imagining.
> The way it is currently you can call Preconditioner::build() to get
> a Preconditioner object appropriate for the current active solver
> type (just like with NumericVector, etc.). This means that when you
> do Preconditioner::build() you will get back a PetscPreconditioner
> object. This object has the ability to use any of the built in
> PetscPreconditioner types. This way we don't have
> ILUPreconditioner, AMGPreconditioner, LUPreconditioner, etc.
> classes... just the one.
> I think you were suggesting we actually bust out those seperate
> classes (unless I'm misunderstanding).
Not necessarily separate classes, just the ability to have separate
classes. Take a look at the TimeSolver stuff: DiffSystem needs a
TimeSolver to integrate; solvers which are similar enough (e.g.
different theta methods) are represented by the same class with
tweakable parameters; solvers which are too different get entirely
And the case of wrapper code for external numerics packages, "too
different" vs "similar enough" depends on how well the underlying
package hides those differences - PETSc is pretty good about OOP, and
it's entirely reasonable to have all their preconditioners wrapped in
a single libMesh class.
> Or maybe that we might still have monolithic Preconditioner classes
> for all the common stuff... and separate ones for the more involved
> (like AMG preconditioners).
AMG is still pretty black-box (that being the whole point) and can
probably just be a type parameter to whatever numerics packages have
that interface - but geometric MG would require enough libMesh code
that we might want to do it as a separate class.
> This is actually where I'd like to go in the future. So I do think
> your suggestion is a good one. The idea would be to make
> Preconditioner::build() take an optional PreconditionerType.
>> Then, when you call set_preconditioner_type it uses that factory to
>> build a new Preconditioner, or when you call set_preconditioner it
>> calls set_preconditioner_type(pre.type()) to keep in sync.
> Agreed... my current version of LinearSolver::set_preconditioner()
You mean LinearSolver::set_preconditioner_type(), right?
> checks to see if a Preconditioner has been attached... and if it has
> then it calls preconditioner->set_preconditioner_type(). Similar
> for get_preconditioner_type, etc.
> I think the only issue is whether or not using a PetscPreconditioner object
> with it's type set to ILU_PRECOND is exactly the same as calling PCSetType()
> and passing in PCILU. The one major difference is that Petsc is going to
> have to call out to an externed function in libMesh that will then translate
> to libmesh NumericVectors and call preconditioner->apply(). So each
> application (and setup) of the preconditioner will incur (probably) more
> work... but it's unclear if it's going to have any actual impact. This is
> really going to come down to testing... I'm just going to have to do some.
> If the speed hit is negligible then I think I'd rather always use
> Preconditioner objects so everything is consistent.
Likewise. That may depend on our respective definitions of