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

bug in IRootFunctions double.Hypot #75651

Closed
c-ohle opened this issue Sep 15, 2022 · 5 comments · Fixed by #75776
Closed

bug in IRootFunctions double.Hypot #75651

c-ohle opened this issue Sep 15, 2022 · 5 comments · Fixed by #75776
Assignees
Milestone

Comments

@c-ohle
Copy link

c-ohle commented Sep 15, 2022

Description

The double.Hypot function returns negative results.

public static double Hypot(double x, double y)
It is in the line result = x + y; where the abs values should be used.

Just as note: I've found that the algorithm used is less precise and slower than a common implementation.

Reproduction Steps

  x = double.Hypot(1, +1e20); // 1E+20 ok
  x = double.Hypot(1, -1e10); // 1E+10 ok
  x = double.Hypot(1, -1e20); // -1E+20 wrong

Expected behavior

  x = double.Hypot(1, +1e20); // 1E+20 ok
  x = double.Hypot(1, -1e10); // 1E+10 ok
  x = double.Hypot(1, -1e20); // 1E+20 

Actual behavior

  x = double.Hypot(1, +1e20); // 1E+20 ok
  x = double.Hypot(1, -1e10); // 1E+10 ok
  x = double.Hypot(1, -1e20); // -1E+20 wrong

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 15, 2022
@ghost
Copy link

ghost commented Sep 15, 2022

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

The double.Hypot functions return negative results.

public static double Hypot(double x, double y)
It is in the line result = x + y; where the abs values should be used.

Just as a note: I've found that the algorithm used is less precise and slower than the simple calculation.

Reproduction Steps

  x = double.Hypot(1, +1e20); // 1E+20 ok
  x = double.Hypot(1, -1e10); // 1E+10 ok
  x = double.Hypot(1, -1e20); // -1E+20 wrong

Expected behavior

  x = double.Hypot(1, +1e20); // 1E+20 ok
  x = double.Hypot(1, -1e10); // 1E+10 ok
  x = double.Hypot(1, -1e20); // 1E+20 

Actual behavior

  x = double.Hypot(1, +1e20); // 1E+20 ok
  x = double.Hypot(1, -1e10); // 1E+10 ok
  x = double.Hypot(1, -1e20); // -1E+20 wrong

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Author: c-ohle
Assignees: -
Labels:

area-System.Numerics, untriaged

Milestone: -

@tannergooding tannergooding added bug and removed untriaged New issue has not been triaged by the area owner labels Sep 16, 2022
@tannergooding tannergooding self-assigned this Sep 16, 2022
@tannergooding tannergooding added this to the 7.0.0 milestone Sep 16, 2022
@jeffhandley
Copy link
Member

This will be considered for backporting to 7.0 since it's a bug in new 7.0 functionality.

@c-ohle
Copy link
Author

c-ohle commented Sep 16, 2022

Is it ok if I report the other NET7 numeric bugs I've found as a collection in a case list with no detailed explanation for each case?
It would save a lot of time.

@tannergooding
Copy link
Member

We need reproduction steps for each of the cases that have bugs so that regression tests can be added.

Not providing them will cause it to take more time overall to get the fix in and risk the fix not making it into .NET 7 RTM.

@c-ohle
Copy link
Author

c-ohle commented Sep 16, 2022

@tannergooding ok, next week.

tannergooding added a commit to tannergooding/runtime that referenced this issue Sep 16, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Sep 16, 2022
tannergooding added a commit that referenced this issue Sep 17, 2022
…quired (#75776)

* Adding a regression test covering #75651

* Ensure that all paths of double.Hypot use the absolute value where required

* Change were the regression tests for Hypot are exposed
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Sep 17, 2022
@dotnet dotnet locked as resolved and limited conversation to collaborators Oct 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants