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. The docstrings in the FooVariationalProblem classes can be expanded to explain the arguments in more detail. -- Anders _______________________________________________ Mailing list: https://launchpad.net/~dolfin Post to : dolfin@lists.launchpad.net Unsubscribe : https://launchpad.net/~dolfin More help : https://help.launchpad.net/ListHelp
