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

Fix logic error in roots.hpp #777

Merged
merged 2 commits into from
Mar 13, 2022
Merged

Fix logic error in roots.hpp #777

merged 2 commits into from
Mar 13, 2022

Conversation

jzmaddock
Copy link
Collaborator

Fixes #776

@jzmaddock jzmaddock merged commit 0bb12b2 into develop Mar 13, 2022
@TFiFiE
Copy link

TFiFiE commented Mar 14, 2022

This fix is itself somewhat problematic insofar that one can no longer reliably test whether the algorithm hit the iteration limit just by checking if the function changed max_iter. It can now also have been decremented by one.

@jzmaddock
Copy link
Collaborator Author

This fix is itself somewhat problematic insofar that one can no longer reliably test whether the algorithm hit the iteration limit just by checking if the function changed max_iter. It can now also have been decremented by one.

I don't follow - the main loop will still decrement max_iter, the change is that now, if there are insufficient iterations left, then we just don't try the alternate bracketing algorithm.

@TFiFiE
Copy link

TFiFiE commented Mar 20, 2022

Before, max_iter didn't get decremented if it ended up being responsible for stopping the algorithm, because count was guaranteed to be decremented at least once in bracket_root_towards_min and bracket_root_towards_max. Although this change might technically be correct, it is now no longer the case that max_iter being changed necessarily implies that it had no hand in stopping the algorithm.

@NAThompson NAThompson deleted the issue776 branch January 24, 2024 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

second_order_root_finder (near) infinite loop
2 participants