← Back to team overview

dolfin team mailing list archive

Re: [HG DOLFIN] Add initial version of new?parameter?system. See test in sandbox/misc.

 

On Saturday 09 May 2009 13:53:06 Anders Logg wrote:
> On Sat, May 09, 2009 at 11:36:23AM +0200, Johan Hake wrote:
> > On Saturday 09 May 2009 00:50:37 Anders Logg wrote:
> > > On Fri, May 08, 2009 at 03:01:18PM +0200, Johan Hake wrote:
> > > > On Friday 08 May 2009 14:57:11 Anders Logg wrote:
> > > > > On Fri, May 08, 2009 at 02:49:01PM +0200, Johan Hake wrote:
> > > > > > On Friday 08 May 2009 14:11:32 DOLFIN wrote:
> > > > > > > One or more new changesets pushed to the primary dolfin
> > > > > > > repository. A short summary of the last three changesets is
> > > > > > > included below.
> > > > > > >
> > > > > > > changeset:   6101:f22b49031d081323d6c6ce18eccbb12c80240b22
> > > > > > > tag:         tip
> > > > > > > user:        Anders Logg <logg@xxxxxxxxx>
> > > > > > > date:        Fri May 08 14:11:28 2009 +0200
> > > > > > > files:       ChangeLog dolfin/parameter/NewParameter.cpp
> > > > > > > dolfin/parameter/NewParameter.h dolfin/parameter/Parameters.cpp
> > > > > > > dolfin/parameter/Parameters.h
> > > > > > > dolfin/parameter/dolfin_parameter.h sandbox/misc/cpp/main.cpp
> > > > > > > description:
> > > > > > > Add initial version of new parameter system. See test in
> > > > > > > sandbox/misc.
> > > > > > >
> > > > > > > Implementation similar to before, but much simplified as a
> > > > > > > result of removing the ParameterValue class and subclassing
> > > > > > > (New)Parameter directly.
> > > > > > >
> > > > > > > Implementation consists of two classes: Parameters and
> > > > > > > (New)Parameter. Only supports int and double so far. Range
> > > > > > > checks implemented.
> > > > > > >
> > > > > > > Check if this looks ok. Will continue to add more features.
> > > > > >
> > > > > > Looks nice. The info was a bit verbose for me, but that's
> > > > > > details.
> > > > >
> > > > > I'm working on an ever more verbose version now, but it's prettier
> > > > > and looks more like what you suggested to me a few days back.
> > > >
> > > > Ok!
> > > >
> > > > > > Will it be possible to
> > > > > >
> > > > > >   my_params.add("another params",my_other_params);
> > > > > >
> > > > > > where my_other_params is a Parameters?
> > > > >
> > > > > Yes, that should be possible to fix. And I guess you mean those
> > > > > should then be nested? So after the above line, one may do
> > > > >
> > > > >   my_params["another params"]["foo"] = 1.0;
> > > >
> > > > Exactly!
> > >
> > > I've implemented this, but I had to change the last [] to ().
> >
> > Looks nice!
>
> Does this look like it will wrap nicely to Python?

Yes I think so. There we can also get rid of the operator()/[] problems we 
face in c++.

We can also add attribute access:

  params.solver.tol = 1e-9

this will only work for parameter names with no white space. We will also be 
able to add some convinient functions like, keys, values, items and 
iter{keys,values,items}.

While addressing whitespace:
Should we have some interpreter for this. Using po together with whitespace 
parameters could cause some trouble.

  app --solver.abs tol 1e-9

vs:

  app --solver.abs_tol 1e-9

> Which features are missing now? I know of two things at least:
>
> 1. Adding value types bool and string

Should parameters of std::vector<Foo> be allowed? Nice for defining markers. 
Here we cannot include any range checks. These should be wrapable to python 
through typemaps.

> 2. Command-line options (using po)

When you have desided on the syntax of the po, see an example above, it would 
be nice to also include a to_option_str function which returns an option 
representation of the Parameters. This is probably only convinent from 
python, where one then can do:

  from app import default_params
  p = default_params()

  jobs = []
  for p1 in [0,1,3,4]:
      p["p1"] = p1
      for p2 in [0,1,2,3,4]:
           p["p2"] = p2
           jobs.append("python app " + p.to_option_str())

  submit(jobs)

> > > Having [] both for nested databases and parameters works fine in
> > > Python but doing it in C++ would mess up the inheritance completely.
> > > I think it's doable but it would complicate the implementation.
> >
> > Is it possible to let NewParameter define a minimal interface, maybe just
> > keep the name of the parameter/parameterlist, then let Parameters and a
> > new class ValueParameter inherit the NewParameter?
>
> I don't think it's possible. The problem is that the thing operator[]
> returns must be something that supports the *full* interface of *both*
> Parameters and NewParameter. Otherwise it will not be possible to do
>
>   // Requires returned object to work as a parameter database
>   parameters["nested parameters"].add("tolerance", 0.01);
>
>   // Requires returned object to work as a parameter
>   parameters["num iterations"] = 1000;
>
> One would then need a dynamic cast on the user side which is not that
> nice. It would be possible to solve this by exposing the full
> interface and returning errors when the wrong functions are called but
> it will make the implementation much less transparent.

Ok.

Johan


Follow ups

References