Skip to content

[newton] fixed bugs with residual being exactly 0#932

Merged
sdrave merged 3 commits into
pymor:masterfrom
HenKlei:fix-newton
Jun 2, 2020
Merged

[newton] fixed bugs with residual being exactly 0#932
sdrave merged 3 commits into
pymor:masterfrom
HenKlei:fix-newton

Conversation

@HenKlei

@HenKlei HenKlei commented May 28, 2020

Copy link
Copy Markdown
Member

Addresses #927.

This merge request fixes a bug that occurred when the residual in the Newton algorithm becomes exactly 0.

@codecov

codecov Bot commented May 28, 2020

Copy link
Copy Markdown

Codecov Report

Merging #932 into master will not change coverage.
The diff coverage is n/a.

Impacted Files Coverage Δ
src/pymor/algorithms/newton.py 86.53% <ø> (ø)

@sdrave sdrave left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the fix, @HenKlei. Maybe it would be more elegant to require atol/rtol to be >= 0 and to let both default to 0? What do you think?

@HenKlei

HenKlei commented May 29, 2020

Copy link
Copy Markdown
Member Author

Yes, I think that's better. That was also my first idea. Then we can remove the first if-condition, I think. As far as I can see, the second one is required nevertheless.

@sdrave

sdrave commented May 29, 2020

Copy link
Copy Markdown
Member

As far as I can see, the second one is required nevertheless.

Don't think so. If error_squence[-1] is not the current but the last error. And if it was zero, then we would have aborted the loop?!

@HenKlei

HenKlei commented May 29, 2020

Copy link
Copy Markdown
Member Author

Don't think so. If error_squence[-1] is not the current but the last error. And if it was zero, then we would have aborted the loop?!

Yeah, you're absolutely right. Having 0 as default value for atol and rtol, that should work.

@HenKlei

HenKlei commented May 29, 2020

Copy link
Copy Markdown
Member Author

I think this is the simplest solution now. The test is also working as expected. Thus, merging is fine with me.
@sdrave Do you have any objections or further comments?

@sdrave

sdrave commented Jun 2, 2020

Copy link
Copy Markdown
Member

Looks good to me! I will merge as is.

@sdrave sdrave merged commit 7deb4a0 into pymor:master Jun 2, 2020
@sdrave sdrave added this to the 2020.1 milestone Jun 2, 2020
@sdrave sdrave added algorithms pr:fix Fixes a bug labels Jun 2, 2020
@HenKlei HenKlei deleted the fix-newton branch June 2, 2020 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:fix Fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants