Skip to content

Sqrt with precision guarantee for GMP#121

Open
volkm wants to merge 6 commits into
stormchecker:masterfrom
volkm:sqrt
Open

Sqrt with precision guarantee for GMP#121
volkm wants to merge 6 commits into
stormchecker:masterfrom
volkm:sqrt

Conversation

@volkm

@volkm volkm commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

Added functions sqrt_precision to compute sqrt with given precision.
For CLN, the use of rationalize should already ensure the precision.
For GMP, we scale both numerator and denominator to achieve the precision.

Fixes stormchecker/storm#873

Comment thread src/carl/numbers/adaption_gmpxx/operations.cpp Outdated
Comment thread src/carl/numbers/adaption_gmpxx/operations.cpp Outdated
Comment thread src/carl/numbers/adaption_gmpxx/operations.cpp Outdated
Comment thread src/carl/numbers/adaption_cln/operations.cpp Outdated
@volkm

volkm commented Mar 4, 2026

Copy link
Copy Markdown
Contributor Author

Speaking of consistent implementations between CLN and GMP: there is also NumberMpq.cpp and NumberCIRA which also have sqrt implementations.

@sjunges Do you know why there are multiple implementations?

@volkm

volkm commented Mar 20, 2026

Copy link
Copy Markdown
Contributor Author

Ready for review.
I will disable the randomized tests before merging.

@sjunges

sjunges commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Hmm, are their big changes that require a re-review? Hard to see from the history :-)

@volkm

volkm commented Apr 25, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, rebase was a bit easier than merging after the formatting touched all files.

It should suffice to look at the current state of the files, the main thing to check is whether the constant factor I added to achieve the precision is actually correct. There is a proof and I also added random tests which did not find any issues.

I will remove the random tests before merging because they significantly increase the testing time.

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.

Very imprecise sqrt computation for rational numbers

3 participants