Bug 147906 - Use hypot function for Pythagorean addition
Summary: Use hypot function for Pythagorean addition
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
7.4.0.0 alpha0+
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.4.0 reviewed:2022 target:7.5...
Keywords: difficultyBeginner, easyHack, skillCpp, skillJava, skillPython, topicCleanup
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2022-03-10 14:42 UTC by Hossein
Modified: 2023-11-19 09:10 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Hossein 2022-03-10 14:42:34 UTC
There are many places in the code that calculating the Pythagorean addition is needed, which is defined as:

    a ⊕ b = sqrt(a^2+b^2)

This is hypotenuse, or the length of the longest side of a right-angled triangle, that we know the other 2 sides, a and b.

https://en.wikipedia.org/wiki/Pythagorean_addition

Theoretically, this calculation could be done using sqrt(a * a + b * b), but there are problems with this approach. For large a or b, there is a possibility of overflow, although the result itself is not that big to cause overflow.

To overcome this problem, there are implementations of hypotenuse that do not use power of 2, and use other methods to calculate the result. One of them which is usable in C++ is the std::hypot. To calculate a ⊕ b, you can easily use std::hypot(a,b).

https://en.cppreference.com/w/cpp/numeric/math/hypot

As an example, you can see this commit from Mike:

https://git.libreoffice.org/core/+/4cbaaf21fe1c22b36dd72798cecfa59e73d0f8c3
How to find instances

Among the result of this output, you can find some instances:

    git grep sqrt

but then you should look for things like 'statement * statement' in the results.
Comment 1 Commit Notification 2022-03-18 17:46:53 UTC
Gautham Krishnan committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/67fcd8f268fd04ca2012470af2257b394b77b8fc

tdf#147906 used std::hypot for Pythagorean addition

It will be available in 7.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 2 Commit Notification 2022-03-23 19:52:49 UTC
VaibhavMalik4187 committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/d0055ac076c61d24c505c2ffa4f99e36c97b1266

tdf#147906 Use std::hypot for Pythagorean addition

It will be available in 7.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 3 Commit Notification 2022-03-28 09:44:30 UTC
offtkp committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/ab9896bfda4d2ef16f3cbb373edced33f9021492

tdf#147906 change all sqrt(a * a + b * b) occurences to std::hypot(a, b)

It will be available in 7.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 4 Bartosz 2022-03-28 13:54:25 UTC
It seems that std::hypot is much slower that square root:
https://stackoverflow.com/questions/32435796/when-to-use-stdhypotx-y-over-stdsqrtxx-yy

I am wondering if this change is worth implementing (possible performance drop).
Comment 5 Commit Notification 2022-03-29 07:18:41 UTC
Bartosz Kosiorek committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/a2d49b680cd026c3d64683f6ee3ba97e9834a7f7

tdf#147906 change sqrt(a * a + b * b) occurences to std::hypot(a, b)

It will be available in 7.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 6 Libreoffice user SSO 2022-03-29 11:02:55 UTC
At core/filter/source/svg/svgwriter.cxx:304 "const double    fRadius = sqrt( static_cast< double >( rObjRect.GetWidth() ) * rObjRect.GetWidth() + rObjRect.GetHeight()*rObjRect.GetHeight() ) * 0.5;" 

Is it a candidate for changing it to: 
"const double fRadius = std::hypot(rObjRect.GetWidth(), rObjRect.GetHeight()) * 0.5"

If yes, then why it's not letting me make a commit containing this lines of code ?

If no, Some explanations will be great :)!!
Comment 7 Hossein 2022-03-29 11:18:03 UTC
(In reply to Libreoffice user SSO from comment #6)
> At core/filter/source/svg/svgwriter.cxx:304 "const double    fRadius = sqrt(
> static_cast< double >( rObjRect.GetWidth() ) * rObjRect.GetWidth() +
> rObjRect.GetHeight()*rObjRect.GetHeight() ) * 0.5;" 
> 
> Is it a candidate for changing it to: 
> "const double fRadius = std::hypot(rObjRect.GetWidth(),
> rObjRect.GetHeight()) * 0.5"
> 
> If yes, then why it's not letting me make a commit containing this lines of
> code ?
The change itself is OK, if you add ; in the end. Let's discuss this in the dev IRC.
Comment 8 Commit Notification 2022-03-31 11:52:17 UTC
pragat-pandya committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/a31aadb1ef7dfdd4ae76a15707ab51a82c95d868

tdf#147906 Use std::hypot for Pythagorean addition

It will be available in 7.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 9 Hossein 2022-06-16 12:58:23 UTC
Re-evaluating the EasyHack in 2022

This issue is still relevant, as there are many places in the code that a Pythagorean addition is done, and std::hypot can be used instead.
Comment 10 Commit Notification 2022-09-03 18:49:14 UTC
Aleksa Savic committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/519dae19abddaedaf0d3d109186eeb4a08471a92

tdf#147906 Use std::hypot for Pythagorean addition

It will be available in 7.5.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2022-11-18 23:49:19 UTC
OmkarAcharekar committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/bcf333309f9a9bde21aac1302cbead2b23822458

tdf#147906 Use std::hypot for Pythagorean addition

It will be available in 7.5.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 12 Commit Notification 2022-11-24 20:56:27 UTC
Bogdan B committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/1e9ace4c35e4206c24ded230433915850a1f04d1

tdf#147906 used std::hypot for Pythagorean addition

It will be available in 7.5.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 13 Commit Notification 2022-11-26 11:13:39 UTC
Bogdan B committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6bf5dd8faea420aa0dbc7c429952c9868bcadb9e

tdf#147906 used std::hypot for Pythagorean addition

It will be available in 7.5.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 14 Commit Notification 2022-11-26 22:40:20 UTC
Bogdan B committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/5b18eebc2c95321ce7e6edf10f4df81557382a48

tdf#147906 used std::hypot for Pythagorean addition

It will be available in 7.5.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 15 Commit Notification 2023-01-09 09:18:59 UTC
Leonid Ryzhov committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/1b44a4c558fdf597bf66c1c671248ca58e145e9e

tdf#147906 Use std::hypot for Pythagorean addition

It will be available in 7.6.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 16 Commit Notification 2023-01-20 12:05:24 UTC
ektagoel12 committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/841ab19fb3f68dbab6295459ef11a257f0f022e8

tdf#147906 Use std::hypot for Pythagorean addition

It will be available in 7.6.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 17 Hossein 2023-01-26 11:11:10 UTC
This EasyHack was previously about C++ std::hypot, but I think it would be fine to use the similar method Math.hypot() from Java, or math.hypot() from Python. Hereby I rename the title to have a more coverage, and I also add the appropriate tag accordingly.
Comment 18 Commit Notification 2023-02-06 18:38:32 UTC
Yomnasalama committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/08cfcea7fef6cde9eddcfa461fe1edff99dadafe

tdf#147906 Use Math.hypot() for Pythagorean addition

It will be available in 7.6.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 19 sunny 2023-03-18 17:26:42 UTC
Hi Hossein

Can you please review my change, gerrit: https://gerrit.libreoffice.org/c/core/+/148547

It's been some time since it's open.
Comment 20 Commit Notification 2023-03-19 18:18:49 UTC
adityasingh22 committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/1f22d01b7ae96a7efa91c9285f3d2f8c37f05237

tdf#147906: Use python's math.hypot() for Pythagorean addition

It will be available in 7.6.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 21 Rishabh Deo Singh 2023-03-23 16:15:12 UTC
I have understood the problem and I want to fix this issue.
Comment 22 Buovjaga 2023-03-23 16:18:42 UTC
Multi-hacker task, don't assign
Comment 23 Commit Notification 2023-03-29 06:01:41 UTC
Yousef_Rabia committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6700f7f5b4eae31bf020e7d073c496c6e67a2397

tdf#147906 Use Math.hypot() for Pythagorean addition

It will be available in 7.6.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 24 Commit Notification 2023-04-23 22:53:11 UTC
buldi committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/c6755955aeaddb14bd3ad231f78cda47a0a5aeaa

tdf#147906 Use std::hypot for Pythagorean addition

It will be available in 7.6.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 25 Commit Notification 2023-08-07 00:50:18 UTC
sahil committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6e12b7f296930c6f37e036407e9fb832d9a9f027

tdf#147906 used std::hypot for Pythagorean addition

It will be available in 24.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 26 Commit Notification 2023-09-21 05:16:19 UTC
apurvapriyadarshi committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/9844a197bfe91e7adb74e9e5859c7fbfaaf99e28

tdf#147906 used StrictMath.hypot for Pythagorean addition

It will be available in 24.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 27 Commit Notification 2023-10-03 06:32:30 UTC
Ankit_Jaipuriar committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/89367021a4c960b6873370ea9472b36457c51ef3

tdf#147906 Use std::hypot for Pythagorean addition

It will be available in 24.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 28 pabhiramrishi 2023-11-19 08:23:40 UTC
Started working on it.
Comment 29 Buovjaga 2023-11-19 09:10:28 UTC
Multi-hacks are not assigned.