dolfin team mailing list archive

["Garth N. Wells" <gnw20@xxxxxxxxx>]

On Mon, Sep 10, 2012 at 1:47 PM, Anders Logg <logg@xxxxxxxxx> 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 <hake.dev@xxxxxxxxx> wrote:
>> > On 09/10/2012 10:55 AM, Garth N. Wells wrote:
>> >> On Mon, Sep 10, 2012 at 9:45 AM, Anders Logg <logg@xxxxxxxxx> wrote:
>> >>> On Sat, Sep 08, 2012 at 11:16:49PM +0200, Johan Hake wrote:
>> >>>>    On Sep 8, 2012 12:04 PM, <[1]noreply@xxxxxxxxxxxxx> wrote:
>> >>>>    >
>> >>>>    > ------------------------------------------------------------
>> >>>>    > revno: 6896
>> >>>>    > committer: Garth N. Wells <[2]gnw20@xxxxxxxxx>
>> >>>>    > 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:noreply@xxxxxxxxxxxxx
>> >>>>    2. mailto:gnw20@xxxxxxxxx
>> >>>>    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@xxxxxxxxxxxxxxxxxxx
>> >>>> Unsubscribe : https://launchpad.net/~dolfin
>> >>>> More help   : https://help.launchpad.net/ListHelp
>> >>>
>> >>>
>> >>> _______________________________________________
>> >>> Mailing list: https://launchpad.net/~dolfin
>> >>> Post to     : dolfin@xxxxxxxxxxxxxxxxxxx
>> >>> Unsubscribe : https://launchpad.net/~dolfin
>> >>> More help   : https://help.launchpad.net/ListHelp
>> >