Skip to content
Snippets Groups Projects

Feature/parameterizedobjectfactory

Merged Christian Engwer requested to merge feature/parameterizedobjectfactory into master

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

Pipeline #1391 passed

Pipeline passed for 446e35bb on feature/parameterizedobjectfactory

Merged by avatar (Mar 20, 2025 7:55am UTC)

Loading

Pipeline #1392 passed

Pipeline passed for 20c53865 on master

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Christian Engwer Added 1 commit:

    Added 1 commit:

    • 37d17762 - [parameterizedobject] allow for non-const parameters
  • Christian Engwer Added 1 commit:

    Added 1 commit:

    • 92b7e366 - [parameterizedobject] use unique_ptr instead of shared_ptr
  • Christian Engwer Added 1 commit:

    Added 1 commit:

    • 47dfcb6a - [parameterizedobject,doc] fix typo
  • Christian Engwer Added 1 commit:

    Added 1 commit:

    • b5a2550a - [parameterizedobject,doc] fix typo
  • Christian Engwer Added 1 commit:

    Added 1 commit:

    • efff03af - [parameterizedobject] use default constructor of no parameters are given
  • Added feature label

  • Christian Engwer Title changed from [WIP] Feature/parameterizedobjectfactory to Feature/parameterizedobjectfactory

    Title changed from [WIP] Feature/parameterizedobjectfactory to Feature/parameterizedobjectfactory

  • The tag parameter seems like an ugly workaround to the shortcomings of introducing global state. IMHO it would be a much cleaner and simpler design to not make everything static and drop the tag.

  • @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.

  • Christian Engwer Added 1 commit:

    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 furthermore have a proposal for a generalization of this class. I'll try if this can be implemented and upload a second feature branch.

  • 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 Engwer
  • It 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 Engwer
  • I'm also not completely happy with this. Using a pointer type also came to my mind, but it seems to be a hack. In fact we never have something like a function returning an Interface or Interface*.

  • I 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 a shared_ptr, as it can be assigned from a unique_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 and unique_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 a map<key,fn> where define(key,creator) forwards to operator[] and create(key,args) mimics [key](args) for the unfortunately non-existing const operator[]`.

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading