Skip to content

fix(math): prevent sqrt_double bisection from stalling - #3264

Open
Reuben006 wants to merge 1 commit into
TheAlgorithms:masterfrom
Reuben006:fix-sqrt-double-convergence
Open

Reuben006 wants to merge 1 commit into
TheAlgorithms:masterfrom
Reuben006:fix-sqrt-double-convergence

Conversation

@Reuben006

Copy link
Copy Markdown

Description of Change

Fix floating-point nontermination in math/sqrt_double.cpp.

The previous implementation could stop making progress before reaching its fixed absolute error tolerance. Input 10000000001 timed out during local testing. The smallest positive subnormal also timed out because the reciprocal workaround overflows.

This change:

  • Normalizes the input using std::frexp before bisection.
  • Stops when rounding makes the midpoint equal an endpoint.
  • Restores the result's scale using std::ldexp.
  • Defines behavior for zero, negative inputs, NaN, and infinity.
  • Adds Doxygen documentation, a worked example, and noninteractive assertion tests.

The implementation still uses bisection. std::sqrt is used only as a test reference; correct rounding is not guaranteed.

Related to #2456.

Validation

  • Debug and optimized C++17 builds passed with strict compiler warnings.
  • Built-in regression and boundary tests passed.
  • Local CMake target build, clang-format, and clang-tidy checks passed.
  • Fork CI passed: Code Formatter and compilation on Ubuntu, Windows, and macOS.
  • An additional local verification harness, outside this PR, checked 99,957 finite positive inputs against std::sqrt, within relative error 2 * std::numeric_limits<double>::epsilon().

Related Work

Checklist

  • Added description of change
  • Existing filename follows repository naming guidelines
  • Added tests and examples; tests pass
  • Added educational Doxygen documentation
  • Updated relevant documentation/comments
  • PR title follows semantic commit guidelines
  • Searched previous suggestions for duplicates
  • I acknowledge that all my contributions will be made under the project's license.

This branch has not been deployed

No deployments
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.

1 participant