dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

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)

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: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
> >