Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving factory parameters #1162

Open
3 of 6 tasks
upsj opened this issue Nov 1, 2022 · 0 comments
Open
3 of 6 tasks

Improving factory parameters #1162

upsj opened this issue Nov 1, 2022 · 0 comments
Labels
is:idea Just a thought - if it's good, it could evolve into a proposal.

Comments

@upsj
Copy link
Member

upsj commented Nov 1, 2022

I think the current factory parameter setup could use with a few improvements:

  • The factory parameters generated by our macros are mutable by default! That's a big code smell IMO, though we can't change it without potentially breaking user code (fixed by make the type in deferred_factory more explicit #1439)
  • The factory parameters get injected into the generated LinOp by default. That means that we need to carry them around and cannot discard them after generation, which may be useful in some cases, like solvers, where we no longer need the preconditioner factory after generation. Again, this can't be changed without an interface break.
  • The fluent interface with_parameter is templated and forwards its argument(s) into a brace-initializer. That means that a parameter of type unsigned can't be initialized with an argument of type int.
  • The current design assumes that there is a 1:1 relationship between LinOpFactories and LinOps. That is a strong limitation, as there are many cases where it makes sense to use a variety of different algorithms (= factories) to generate the same kind of object, e.g. factorizations. Add sparse direct interface #1082
  • Redundancy: We specify the same stopping_criterion, preconditioner, initial_guess, logger for all iterative solvers. What if we built a factory parameter type hierarchy that mirrors the underlying solver hierarchy? That way, users have a single place where to look up how to set up a solver, and we only need to maintain a single piece of documentation. Most solvers will not need any additional parameters, and the ones that do will only need a handful of parameters, like the restart in GMRES. Handled by Factory improvements #1336
  •  Potentially it might make sense to decouple the fluent interface more from the parameters, to allow different ways of specifying the same parameter or similar.

As a first step, I built #1082 with suggested fixes for most of these issues in mind.

@upsj upsj added this to the Ginkgo 2.0.0 milestone Nov 1, 2022
@upsj upsj removed this from the Ginkgo 2.0.0 milestone Nov 2, 2022
@upsj upsj mentioned this issue Nov 7, 2022
2 tasks
@upsj upsj added the is:idea Just a thought - if it's good, it could evolve into a proposal. label Nov 23, 2022
@upsj upsj mentioned this issue May 10, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is:idea Just a thought - if it's good, it could evolve into a proposal.
Projects
None yet
Development

No branches or pull requests

1 participant