dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.27k stars 4.73k forks source link

Math.Atan bug #45841

Closed Monclops123 closed 3 years ago

Monclops123 commented 3 years ago

I'm not sure if this is the right place to post this, but I'm running .NET Framework 4.7.2 and I'm getting a weird bug where Math.Atan sometimes returns the wrong value. Each time I've tested it, the value returned should be -1.5707 but sometimes it returns 1.5707.

double rise = point2.Y - point1.Y; rise = Math.Round(rise * Math.Pow(10, 4)) / Math.Pow(10, 4); double run = point2.X - point1.X; run = Math.Round(run * Math.Pow(10, 4)) / Math.Pow(10, 4); double angle = Math.Atan(rise / run);

When testing this the rise was always negative and the run was zero (I checked to make sure it wasn't just really small).

danmoseley commented 3 years ago

Can you see whether it repros in.NET 5?

cc @tannergooding

Monclops123 commented 3 years ago

I can only select a .NET Framework and the most recent one I can find is 4.8.

danmoseley commented 3 years ago

You can install.NET 5 here : https://dotnet.microsoft.com/download/dotnet/5.0

Clockwork-Muse commented 3 years ago

When testing this the rise was always negative and the run was zero

So, when the value of the math was -Infinity. As an effort to reproduceability, do you have a table of input points and the resulting rise/run values? Note that if rise is positive than the intermediate value should be +Infinity.

As a side note, apparently the string representation used to be "∞" on .NET Framework, instead of the string "Infinity"...

Monclops123 commented 3 years ago

@danmosemsft I'm sorry, I'm still a bit new to all of this; I'm using Visual Studio and when I go to build my project I only have the option to select a Framework. Should I see .NET options in the dropdown?

@Clockwork-Muse I took a screenshot of each scenario and I included the run before it was rounded because it appears like that might be the value it's using. (These points are directly from my testing in Revit) image image

Clockwork-Muse commented 3 years ago

... dotnetfiddle is getting the "correct value", at least:

rise: -1.9544
run: 0
angle: -1.5707963267949

How about explicitly printing the values out using Console.WriteLine(whateverValue.ToString("R");? Although it appears that Revit is displaying the round-trip value already...

SingleAccretion commented 3 years ago

@Monclops123 It would probably be easier for you to just create a new project that targets .NET 5. Here are the steps: open VS -> click "Create new project" -> enter ".NET 5" in the search box and choose "Console App (.NET Core)" -> "Next" -> choose the name and location for the project -> "Create" -> open properties of the project -> change "Target Framework" to .NET 5.

Unfortunately this experience is not really ideal right now, but there are epics to improve it and I do hope they are implemented in v6.

ghost commented 3 years ago

Tagging subscribers to this area: @tannergooding, @pgovind See info in area-owners.md if you want to be subscribed.

Issue Details
I'm not sure if this is the right place to post this, but I'm running .NET Framework 4.7.2 and I'm getting a weird bug where Math.Atan sometimes returns the wrong value. Each time I've tested it, the value returned should be -1.5707 but sometimes it returns 1.5707. `double rise = point2.Y - point1.Y;` `rise = Math.Round(rise * Math.Pow(10, 4)) / Math.Pow(10, 4);` `double run = point2.X - point1.X;` `run = Math.Round(run * Math.Pow(10, 4)) / Math.Pow(10, 4);` `double angle = Math.Atan(rise / run);` When testing this the rise was always negative and the run was zero (I checked to make sure it wasn't just really small).
Author: Monclops123
Assignees: -
Labels: `area-System.Numerics`
Milestone: -
tannergooding commented 3 years ago

Given this is .NET Framework, you will need to use BitConverter.DoubleToInt64Bits(value).ToString("X") and potentially value.ToString("G17")

Full framework will not always produce roundtrippable values and will not correctly handle display of certain values such as -0.0 which can impact the end result: https://devblogs.microsoft.com/dotnet/floating-point-parsing-and-formatting-improvements-in-net-core-3-0/.

Getting the raw bits and printing the hex result allows you to guarantee you know the sign and raw values are the same. Using G17 generally gives a unique value outside some edge cases that were fixed in .NET Core 3.0+.

If run is merely differing on +0.0 vs -0.0, that would explain the sign flipping in various scenarios.

danmoseley commented 3 years ago

@tannergooding could you clarify what change you're proposing to @Monclops123 code above?

Monclops123 commented 3 years ago

@SingleAccretion Thanks for the walkthrough! However, in order for this to work with Revit I need to create the dll file like the Class Library does and add the RevitAPI.dll as a reference. It looks like I can create a Class Library with .NET 5 but I'm not sure how that would work with the tools I've created, especially when some of them use WinForms.

SingleAccretion commented 3 years ago

@Monclops123 Hmm, I see. My recommendation then would be the same as Tanner's: to get the raw bits for run. Flipping between -0.0 (-9223372036854775808 as long) and +0.0 (0 as long) would indeed explain what you're seeing.

tannergooding commented 3 years ago

@danmosemsft, just that for the purposes of debugging it may be beneficial to add, for example, BitConverter.DoubleToInt64Bits(run).ToString("X") in the watch window. This would allow them to see whether 0 is actually +0.0 or -0.0 (one will be 0, the other 0x8000000000000000). The same can be done for other values to ensure the bits are as they expect.

Likewise, rise.ToString("G17"), for example, may show differences in the values that are hidden by "R" or the default formatting scheme, but in a more human readable format.

danmoseley commented 3 years ago

Ah right, for debugging. I expect your hypothesis is correct and I was wondering what they could do to get a stable value.

danmoseley commented 3 years ago

It looks like I can create a Class Library with .NET 5 but I'm not sure how that would work with the tools I've created, especially when some of them use WinForms.

@Monclops123 Note that Winforms also works with .NET 5 (I'm not suggesting you start rebasing your app/tools on .NET 5, just sharing this info which may be relevant to your future projects)

Monclops123 commented 3 years ago

@danmosemsft Thanks for the tip! Is there a difference between .NET and .NET Core? I tried doing a little research and someone said .NET Core doesn't support WinForms, unless that was added with the update?

@tannergooding Thank you for clarifying, I just ran the debugger again and that seems to be exactly what's happening. Now I just need to figure out how to correct it. I imagine multiplying by -1 would work if it's negative?

danmoseley commented 3 years ago

@Monclops123 in brief - .NET Core is the future of .NET. .NET Framework will be supported for a very long time, but it is not getting new features. .NET Framework 4.8 is the last version of .NET Framework. .NET 5 is the newest version of .NET Core (we dropped the Core suffix after 3.1). NET Core 3.x/.NET 5 and later do support WinForms however the Visual Studio Designer does not yet have as many features. Although .NET Core/.NET 5 supports Unix, WinForms remains Windows specific, but using .NET Core/.NET 5 can give you higher performance and new features. There is some info to get started here and from https://dotnet.microsoft.com/. https://github.com/dotnet/winforms is a great place to get more detailed answers if you are interested.

Monclops123 commented 3 years ago

@danmosemsft Thank you so much!

Also, I managed to get the results I was expecting. Thank you all so much for the help!