dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Mon, Jul 04, 2011 at 11:28:56PM +0100, Garth N. Wells wrote:
>
>
> On 04/07/11 17:22, Anders Logg wrote:
> > On Mon, Jul 04, 2011 at 04:47:46PM +0100, Garth N. Wells wrote:
> >>
> >>
> >> On 04/07/11 16:44, Anders Logg wrote:
> >>> On Mon, Jul 04, 2011 at 04:39:04PM +0100, Garth N. Wells wrote:
> >>>> I'm not sold on the NonlinearVariationalProblem interface. I would
> >>>> prefer a constructor takes the Jacobian as an argument. It's much
> >>>> cleaner to do things at construction and removes the need to later
> >>>> attach the Jacobian.
> >>>
> >>> The point is that one should be able to define a nonlinear problem
> >>> with or without a Jacobian. Not all nonlinear solvers need a Jacobian.
> >>>
> >>
> >> That's why I wrote 'a' constructor. We can have two versions.
> >
> > Yes, that's an option. The drawback with that is that it would double
> > the number of constructors (from 6 to 12) but it's a small thing to
> > fix. I wouldn't mind moving it to the constructor.
> >
>
> I think that we can rationalise the number of constructors in
> FooVariationalProblem. For the shared pointer versions, it would be
> enough to have just
>
>     LinearVariationalProblem(boost::shared_ptr<const Form> a,
>          boost::shared_ptr<const Form> L,
>          boost::shared_ptr<Function> u,
>          std::vector<boost::shared_ptr<const BoundaryCondition> > bcs);
>
> and
>
>     NonlinearVariationalProblem(boost::shared_ptr<const Form> F,
>          boost::shared_ptr<const Form> J,
>          boost::shared_ptr<Function> u,
>          std::vector<boost::shared_ptr<const BoundaryCondition> > bcs);

I think it's important that we keep the same argument order as for the
reference versions, and that the Jacobian comes last to emphasize
that it is an optional/auxiliary argument. It also removes some of the
confusion we had be for with (a, L) vs (F, J). How about this:

  LinearVariationalProblem(boost::shared_ptr<const Form> a,
                           boost::shared_ptr<const Form> L,
                           boost::shared_ptr<Function> u,
                           std::vector<boost::shared_ptr<const
                           BoundaryCondition> > bcs);

  NonlinearVariationalProblem(boost::shared_ptr<const Form> F,
                              int rhs,
                              boost::shared_ptr<Function> u,
                              std::vector<boost::shared_ptr<const
                              BoundaryCondition> > bcs,
                              boost::shared_ptr<const Form> J);

Same as your suggestion (just one shared ptr constructor in each
class) but placing the Jacobian last and keeping the right-hand side
in there.

Keeping the right-hand side is important since it makes it possible to
check for errors (like a nonzero right-hand side) in one single
place (inside NonlinearVariationaProblem.cpp). We would otherwise need
to check it inside Equation.cpp and possibly in the Python layer.

--
Anders


> Garth
>
> >>> I agree that it's in general cleaner to require as much data as
> >>> possible at the time of construction, but think that the handling of
> >>> the Jacobian data is quite clean: it's a shared pointer that may be
> >>> null and the nonlinear solver can call has_jacobian to check whether
> >>> it has been specified.
> >>>
> >>
> >> Which we can still do with two constructors.