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

Add initial_guess parameter to apply_inverse #941

Merged
merged 1 commit into from Jun 8, 2020

Conversation

sdrave
Copy link
Member

@sdrave sdrave commented Jun 5, 2020

and apply_inverse_adjoint.

It is used in the following places:

  • Operator.apply_inverse passes initial_guess to newton in case of a nonlinear operator.
  • Generic lgmres, as well as SciPy bicgstab, lgmres, lsmr, lsqr.
  • The FEniCS bindings pass initial_guess to the backend.
  • implicit_euler sets initial_guess to the solution of the last time step.

Fixes #924.

@sdrave sdrave added pr:new-feature Introduces a new feature interfaces labels Jun 5, 2020
@sdrave sdrave added this to the 2020.1 milestone Jun 5, 2020
@sdrave sdrave requested a review from pmli June 5, 2020 10:48
@pmli
Copy link
Member

pmli commented Jun 5, 2020

Looks good to me.
I'm just wondering, since it helps with instationary models, could/should the initial_guess also be passed to stationary models?

@codecov
Copy link

codecov bot commented Jun 5, 2020

Codecov Report

Merging #941 into master will decrease coverage by 0.57%.
The diff coverage is 88.79%.

Impacted Files Coverage Δ
src/pymor/operators/mpi.py 17.56% <50.00%> (-0.25%) ⬇️
src/pymor/operators/list.py 64.35% <77.77%> (+0.35%) ⬆️
src/pymor/operators/block.py 83.88% <85.71%> (-0.51%) ⬇️
src/pymor/operators/numpy.py 78.60% <85.71%> (+0.10%) ⬆️
src/pymor/operators/constructions.py 83.89% <90.47%> (+0.13%) ⬆️
src/pymor/algorithms/genericsolvers.py 78.35% <100.00%> (+0.08%) ⬆️
src/pymor/algorithms/timestepping.py 78.18% <100.00%> (ø)
src/pymor/bindings/fenics.py 81.57% <100.00%> (ø)
src/pymor/bindings/ngsolve.py 85.29% <100.00%> (ø)
src/pymor/bindings/pyamg.py 86.36% <100.00%> (+0.31%) ⬆️
... and 3 more

@sdrave
Copy link
Member Author

sdrave commented Jun 5, 2020

I'm just wondering, since it helps with instationary models, could/should the initial_guess also be passed to stationary models?

But how to choose an initial guess for the stationary case?

@pmli
Copy link
Member

pmli commented Jun 5, 2020

I guess this is something only the user could provide. My question is how to give this possibility to the user. (I admit it's kind of a different issue from adding initial_guess to apply_inverse.)

@sdrave
Copy link
Member Author

sdrave commented Jun 8, 2020

I guess this is something only the user could provide. My question is how to give this possibility to the user. (I admit it's kind of a different issue from adding initial_guess to apply_inverse.)

I think it should be the job of the Model to set an adequate initial guess if needed. It is not hard to subclass StationaryModel and implement your own _solve. It would also be imaginable to extend StationaryModel with an additional __init__ argument or an initial_guess method. But before doing so, I would like to have a user who has a need for it.

Copy link
Member

@pmli pmli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, sounds good.

@sdrave sdrave merged commit 8eec9ea into master Jun 8, 2020
@sdrave sdrave deleted the apply_inverse_initial_guess branch June 8, 2020 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:new-feature Introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Initial guess parameter for apply_inverse
2 participants