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

Repace 'invert_options' by 'solver_options' (second try) #184

Merged
merged 5 commits into from Nov 19, 2015

Conversation

Projects
None yet
2 participants
@sdrave
Copy link
Member

sdrave commented Nov 18, 2015

This pull request contains the following changes:

  • options is removed from OperatorInterface.apply_inverse
  • invert_options is removed from OperatorInterface
  • solver_options is added to OperatorInterface
  • solver_options can be given to the implict Euler algorithm.

If not None, solver_options is a dict which can contain the keys
'inverse' and 'jacobian'. The following rules apply:

  • OperatorInterface.apply_inverse uses the 'inverse' entry
    of solver_options for solver selection and configuration.
    If not available, or None, defaults are used.
  • OperatorInterface.jacobian uses the values of the 'jacobian'
    entry of solver_options as solver_options for the Jacobian
    operator. If missing, solver_options is set to None.
  • OperatorInterface.assemble returns an assembled operator with
    the same solver_options.
  • OperatorInterface.projected and OperatorInterface.restricted
    return operators with solver_options set to None.

This PR addresses #122.

sdrave added some commits Nov 18, 2015

[operators] introduce 'solver_options'
- 'options' is removed from OperatorInterface.apply_inverse
- 'invert_options' is removed from OperatorInterface
- 'solver_options' is added to OperatorInterface

If not 'None', solver_options is a dict which can contain the keys
'inverse' and 'jacobian'. The following rules apply:

- OperatorInterface.apply_inverse uses the 'inverse' entry
  of solver_options for solver selection and configuration.
  If not available, or 'None', defaults are used.
- OperatorInterface.jacobian uses the values of the 'jacobian'
  entry of solver_options as solver_options for the Jacobian
  operator. If missing, solver_options is set to None.
- OperatorInterface.assemlbe returns an assembled operator with
  the same solver_options.
- OperatorInterface.projected and OperatorInterface.restricted
  return operators with solver_options set to None.
@sdrave

This comment has been minimized.

Copy link
Member Author

sdrave commented Nov 18, 2015

After some discussion, we decided that there are too many corner cases where the approach in #183 does not work out very well.

As there are relatively few places, where new operators are created, for which appropriate solver_options cannot be known at this point, it seems better to make the solver_options used for these new operators configurable (see 1e54111).

Merge branch 'master' into solver_options2
Conflicts:
	src/pymor/algorithms/timestepping.py
@ftalbrecht

This comment has been minimized.

Copy link
Member

ftalbrecht commented Nov 19, 2015

Very nice, thank you! If I remember correctly, we also decided to allow for an 'adjoint_inverse' key in the 'solver_options', just for future reference.

The tests currently seem to fail bc. of the newton, which might be related to 45c797f?

sdrave added a commit that referenced this pull request Nov 19, 2015

Merge pull request #184 from pymor/solver_options2
Repace 'invert_options' by 'solver_options'

@sdrave sdrave merged commit e1301a8 into master Nov 19, 2015

1 of 2 checks passed

continuous-integration/travis-ci/push The Travis CI build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sdrave sdrave deleted the solver_options2 branch Nov 19, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment