On Mon, Sep 10, 2012 at 1:47 PM, Anders Logg <[email protected]> wrote: > 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 <[email protected]> wrote: >> > On 09/10/2012 10:55 AM, Garth N. Wells wrote: >> >> On Mon, Sep 10, 2012 at 9:45 AM, Anders Logg <[email protected]> wrote: >> >>> On Sat, Sep 08, 2012 at 11:16:49PM +0200, Johan Hake wrote: >> >>>> On Sep 8, 2012 12:04 PM, <[1][email protected]> wrote: >> >>>> > >> >>>> > ------------------------------------------------------------ >> >>>> > revno: 6896 >> >>>> > committer: Garth N. Wells <[2][email protected]> >> >>>> > 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) >
That is at runtime. Not when writing code. > 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. Unnecessary use of string-base naming over methods over explicit, named members and data is a poor design and makes working with code confusing. > I very strongly think we should use > the parameter system and not start to clutter the classes with flag > member variables. > Clutter? It's four lines of code, and a lot more than that was removed. Using parameters adds more garbage. I strongly oppose using the parameter system in this case because it's pointless. If someone else wants to clean up the classes and implement all the functionality that I need (and they do it this week), then I could live with the use of parameters ;). Garth > -- > 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:[email protected] >> >>>> 2. mailto:[email protected] >> >>>> 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 : [email protected] >> >>>> Unsubscribe : https://launchpad.net/~dolfin >> >>>> More help : https://help.launchpad.net/ListHelp >> >>> >> >>> >> >>> _______________________________________________ >> >>> Mailing list: https://launchpad.net/~dolfin >> >>> Post to : [email protected] >> >>> Unsubscribe : https://launchpad.net/~dolfin >> >>> More help : https://help.launchpad.net/ListHelp >> > _______________________________________________ Mailing list: https://launchpad.net/~dolfin Post to : [email protected] Unsubscribe : https://launchpad.net/~dolfin More help : https://help.launchpad.net/ListHelp

