On Mon, Sep 10, 2012 at 11:10:30AM +0100, Garth N. Wells wrote: > On Mon, Sep 10, 2012 at 10:54 AM, Johan Hake <hake....@gmail.com> wrote: > > On 09/10/2012 10:55 AM, Garth N. Wells wrote: > >> On Mon, Sep 10, 2012 at 9:45 AM, Anders Logg <l...@simula.no> wrote: > >>> On Sat, Sep 08, 2012 at 11:16:49PM +0200, Johan Hake wrote: > >>>> On Sep 8, 2012 12:04 PM, <[1]nore...@launchpad.net> wrote: > >>>> > > >>>> > ------------------------------------------------------------ > >>>> > revno: 6896 > >>>> > committer: Garth N. Wells <[2]gn...@cam.ac.uk> > >>>> > branch nick: assembler > >>>> > timestamp: Sat 2012-09-08 10:49:23 +0100 > >>>> > message: > >>>> > Start cleaning up assemblers. > >>>> > > >>>> > The assembler classes are no longer full of static member > >>>> functions > >>>> (this was pointless because we have free function for easy access) and > >>>> the host of optional boolean arguments have been removed from the > >>>> member function interfaces and made part of a common base class. > >>>> > > >>>> > Simple usuage remains unchanged. For more advanced usage, > >>>> FooAssembler object should be created and the boolean options set via > >>>> > > >>>> > assmebler.reset_tensor = false; > >>>> > > >>>> > etc. This should be much more intelligible and less error prone. > >>>> > renamed: > >>>> > >>>> Nice! > >>> > >>> Yes nice, but the parameter system should be used as for other classes: > >>> > >>> assembler.parameters["reset_tensor"] = false; > > > > ++ > > > >> I don't think so. There is no advantage to using parameters in this > >> case. It just adds complexity. > > > > What complexity more than having to deal with an extra parameters type > > instead of the bools attributes? > > > > Yes, more code = increased complexity.
It's a very small price to pay for having a uniform interface. We use it in all other places. > > The whole thing with parameters attached to objects is that these can > > then be nested into other parameters easily. > > > > I can't see any reason why one would want to do that. The settings are > specific to the object. No. One might want to control some of the settings from say NonlinearVariationalSolver. > Members functions/data should be preferable over parameters when > sensible. It's explicit to see a named member function or data, unlike > trawling through code to find allowable parameters based on a string. > And one gets compile time checking. No. Parameter names can be easily listed via info(assembler.parameters, True) Then one gets a complete list of which parameters can be set, including possible values for the parameters. We also get automatic range checking. > Parameters are suitable for options that are passed on the command > line, are optional and/or might be shared amongst objects. > > Using strings over members functions/data unnecessarily is a > nightmare. It took a lot over work to untangle the crazy string-based > storage that was used for parallel mesh data so that we could move the > parallel meshes forward. I don't see how that is relevant. I very strongly think we should use the parameter system and not start to clutter the classes with flag member variables. -- Anders > Garth > > > Johan > > > >> Garth > >> > >>> > >>> > >>>> Johan > >>>> > >>>> > dolfin/fem/AssemblerTools.cpp => dolfin/fem/AssemblerBase.cpp > >>>> > dolfin/fem/AssemblerTools.h => dolfin/fem/AssemblerBase.h > >>>> > modified: > >>>> > demo/undocumented/periodic/cpp/main.cpp > >>>> > demo/undocumented/smoothing/python/demo_smoothing.py > >>>> > dolfin/ale/HarmonicSmoothing.cpp > >>>> > dolfin/fem/Assembler.cpp > >>>> > dolfin/fem/Assembler.h > >>>> > dolfin/fem/LinearVariationalSolver.cpp > >>>> > dolfin/fem/OpenMpAssembler.cpp > >>>> > dolfin/fem/OpenMpAssembler.h > >>>> > dolfin/fem/SymmetricAssembler.cpp > >>>> > dolfin/fem/SymmetricAssembler.h > >>>> > dolfin/fem/SystemAssembler.cpp > >>>> > dolfin/fem/SystemAssembler.h > >>>> > dolfin/fem/assemble.cpp > >>>> > dolfin/fem/dolfin_fem.h > >>>> > dolfin/swig/modules/fem/dependencies.txt > >>>> > dolfin/swig/modules/fem/module.i > >>>> > site-packages/dolfin/compilemodules/swigimportinfo.py > >>>> > dolfin/fem/AssemblerBase.cpp > >>>> > dolfin/fem/AssemblerBase.h > >>>> > The size of the diff (1283 lines) is larger than your specified > >>>> limit > >>>> of 500 lines > >>>> > > >>>> > > >>>> > Your team DOLFIN Core Team is subscribed to branch lp:dolfin. > >>>> > To unsubscribe from this branch go to > >>>> > >>>> [4]https://code.launchpad.net/~dolfin-core/dolfin/trunk/+edit-subscript > >>>> ion > >>>> > >>>> Referenser > >>>> > >>>> 1. mailto:nore...@launchpad.net > >>>> 2. mailto:gn...@cam.ac.uk > >>>> 3. https://code.launchpad.net/~dolfin-core/dolfin/trunk > >>>> 4. > >>>> https://code.launchpad.net/~dolfin-core/dolfin/trunk/+edit-subscription > >>> > >>>> _______________________________________________ > >>>> Mailing list: https://launchpad.net/~dolfin > >>>> Post to : dolfin@lists.launchpad.net > >>>> Unsubscribe : https://launchpad.net/~dolfin > >>>> More help : https://help.launchpad.net/ListHelp > >>> > >>> > >>> _______________________________________________ > >>> Mailing list: https://launchpad.net/~dolfin > >>> Post to : dolfin@lists.launchpad.net > >>> Unsubscribe : https://launchpad.net/~dolfin > >>> More help : https://help.launchpad.net/ListHelp > > _______________________________________________ Mailing list: https://launchpad.net/~dolfin Post to : dolfin@lists.launchpad.net Unsubscribe : https://launchpad.net/~dolfin More help : https://help.launchpad.net/ListHelp
