dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Tue, Jul 05, 2011 at 07:49:51AM +0100, Garth N. Wells wrote:
>
>
> On 05/07/11 07:24, Anders Logg wrote:
> > On Mon, Jul 04, 2011 at 11:46:00PM +0100, Garth N. Wells wrote:
> >>
> >>
> >> On 04/07/11 23:37, Anders Logg wrote:
> >>> 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.
> >>>
> >>
> >> I'm don't mind the order being the same (I didn't pay any attention to
> >> it). My main point is that the shared_ptr interface is lower level so we
> >> don't need to provide multiple convenience versions.
> >
> > Agree.
> >
> >>> 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.
> >>>
> >>
> >> I don't like it. It makes the nonlinear interface clumsy. Reading
> >> the signature it's not clear to me what it's for or what I should
> >> pass in.
> >
> > The point is that it makes the interface for all variational problems
> > (linear or nonlinear) the same:
> >
> >   lhs, rhs, solution, [bcs], [jacobian]
> >
> > I think this is helpful to minimize errors.
> >
>
> I don't agree. It's confusing. It's not clear to me why I would want to
> pass a pointless integer to a NonlinearVariationalProblem. I wouldn't
> expect a pointless argument, so I would start to wonder what it should
> be and what it's for.

Looks like this is a matter of taste. Other opinions?

--
Anders