Feature/parameterizedobjectfactory
the idea is to have a generic factory which can be extended by the user at run time and maps a key (usually an std::string
) to a constructor-call (usually taking a Dune::ParameterTree
).
We want something like a plugin-system, to instantiate implementations of an abstract interface. Usually a factory can only create a hard-coded list of objects. The proposed ParameterizedObjectFactory different modules (or the user) can register implementations of the interface, without modifying the factory code.
Example:
using V = Dune::BlockVector<Dune::FieldVector<double,2>>;
using M = Dune::BCRSMatrix<Dune::FieldMatrix<double,2,2>>;
using PreconditionerFactory =
Dune::ParameterizedObjectFactory<Dune::Preconditioner<V,V>(const M&, const Dune::ParameterTree&)>;
PreconditionerFactory::define<Dune::SeqGS<M,V,V>>("Gauss-Seidel");
Merge request reports
Activity
Added 1 commit:
- 37d17762 - [parameterizedobject] allow for non-const parameters
Added 1 commit:
- 92b7e366 - [parameterizedobject] use unique_ptr instead of shared_ptr
Added 1 commit:
- b5a2550a - [parameterizedobject,doc] fix typo
Added 1 commit:
- efff03af - [parameterizedobject] use default constructor of no parameters are given
Added feature label
@carsten.graeser, perhaps I don't understand you correctly, but everything is
static
. This is exactly the reason for the tag parameter. With this parameter I can distinguish different factories for the base class.Imagine a user code where I want to specify dynamically a source-function and a Dirichlet-extension. Both are some typo of global function mapping from \Omega to R and thus both will end up in the same factory, as long as I don't separate them explicitly. I do this by the tag parameter. Do you have other suggestion?
Sure, we could also ignore this problem... the user should know which one is the correct on, on the other hand, if both are referenced by the name of the example?
An alternative might be to add some type of namespace to interface, similar to the hierarchy in the ParameterTree. We could then prefix the key, iff the key is a string. In this case I think we should hard-code the key to be a string.
As I said I'm asking why not just make things non-static. Then you could simply use different factory objects for different needs. If you absolutely want to have global ones, because you don't want to pass them around, you can still store your instances globally.
Also the tags only solve the issue partially, because one may want different factories, where they cannot be distinguished statically.
Added 1 commit:
- 5dabbf84 - [parameterizedobject] remove singleton object
@carsten.graeser thanks for the input. You are right. It is easy enough to create a singleton outside the factory. I updaten the branch accordingly.#
More comments? Otherwise I'll merge, as Linus wants to build upon this branch for his istl work.
Your approach does not compile with gcc-4.9 because it does not allow function types with abstract return type, i.e., Interface(Args...) is not allows. As a remedy one may pass the Interface and Args types seperately. Unfortunately one cannot have the default key type then unless one packs the Args... pack into a type like, e.g., TypeList in dune-function. However this would be even less elegant to specifying the key type explicitly.
Attached you can find a patch that splits the arguments and also removes the *Base class which is useless since the tag parameter is gone.0001-Avoid-function-type-with-abstract-return-type.patch
I'm not totally happy with your proposed patch, as it forces me to specify the string explicitly. Usually it will be string and it would be much more convenient, if this type could have default value.
I'll look into the gcc-4.9 thing.
Edited by Christian EngwerIt works also in gcc-4.9 if we use a pointer in the signature:
Dune::ParameterizedObjectFactory<InterfaceB*(int, std::string)> FactoryB; ^^^
instead of
Dune::ParameterizedObjectFactory<InterfaceB(int, std::string)> FactoryB;
(and obviously the template specialization has to be adjusted.
Edited by Christian EngwerI just pushed a the branch feature/flexibleparameterizedobjectfactory (8e70c875). The main change is that the return type of create is no longer tied to unique_ptr but is just T. If you register an Impl this automatically selects a proper creator function. Depending on T this will call make_shared, make_unique, or a constructor. The impact on user code is that you must now explicitly use unique_ptr instead of Interface as template parameter.
This allows to use this class also with shared_ptr or types with value semantics. Furthermore the branch adds support for providing custom creators. This is can be used via lambdas that allow to adapt a non-fitting type or to return a pre-created object stored in a closure. See the test for examples.
As a side-effect we can use the signature notation and a default key type again.
Hi Carsten, I looked at your branch.
What I like is the possibility to use a different construction call, in case that the constructor does not patch the required interface.
I'm not sure we really want all this magic to have the different storage types. If you want a
shared_ptr
you can just assign the result to ashared_ptr
, as it can be assigned from aunique_ptr
. Immediately creating the object does not actually fit into the context, as we have virtual interfaces and they don't work via objects. Did you test this? I don't get how it should work. You will alway slice to the base class and loose the desired functionality. I'm sure I overlook a particular use-case, but currently I don't see it.Regarding the syntax, I'm currently not sure what we want to have... I'm still thinking about the different possibilities. Using the
Hi Christian, I added an example with value semantics to the test in my branch (28a50ce2).
Assume that you want a factory to provide functions for rhs, dirichlet data, coefficients, etc. Then you don't have a polymorphic but a type erased interface class like std::function and the conversion does not slice but wrap the created objects.
I'm also not convinced about the magic when storing an object. We may leave writing the lambda for this special case to the user. The other two cases,
shared_ptr
andunique_ptr
, are just there to keep the simple usage of your original proposal.BTW: If we drop them, too, and let the user write the one-line lambda forwarding to
make_unique
, then the whole class is just amap<key,fn>
wheredefine(key,creator)
forwards tooperator[]
andcreate(key,args)
mimics[key](args)
for the unfortunately non-existingconst
operator[]`.