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

Allow the Math methods to return float values #14000

Closed
xanather opened this issue Jan 21, 2015 · 8 comments
Closed

Allow the Math methods to return float values #14000

xanather opened this issue Jan 21, 2015 · 8 comments
Assignees
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Runtime
Milestone

Comments

@xanather
Copy link

I find myself casting returned results from Math methods into floats constantly, even if only floats are supplied as parameters for that method. Many developers don't need the accuracy of a double and floats works fine, many other numeric structs store their values are floats too (such as a Vector2). While explicit casting doesn't hurt anyone there are other classes I have come by that return a float if the supplied parameter(s) is float only.

This wouldn't really be considered a new API addition, all it would do is add overloaded methods that accepts floats as parameters and returns a float (could call the Double version of the Method underneath to avoid code duplication and cast back). If any of the parameters become a double then the double version of the Method would be called and a double value would return.

I would like feedback on what other people think before implementing it.

@mikedn
Copy link
Contributor

mikedn commented Jan 21, 2015

I'd like to see these overloads too but you can't really claim that this isn't a new API addition. Adding overloads can break existing code and as such this will probably need a proper API review.

The way C# treats such overloads is problematic. The following code prints 2147483647.00:

static double f(double d) {
    return d;
}
static void Main() {
    Console.WriteLine("{0:f}", f(int.MaxValue));
}

But if you add a float overload to f then the result changes to 2147484000.00. That's because the float overload gets picked and a float can't accurately represent int.MaxValue. Now, it's quite uncommon to see code that calls methods like Math.Cos with integer arguments but this shows that adding overloads can cause problems.

It's also worth discussing if such overloads should really be implemented by calling the double versions and casting back to float. For functions like sin and cos you may get better efficiency if you compute the result at float precision directly. Of course, that would mean that the runtime would have to add support for such overloads...

@xanather
Copy link
Author

@mikedn Never even thought about seeing how integers would work in a scenario like this. Your right, this change would disallow the possibility of doubles from being returned if using an int. As an alternative could be a new API e.g.

Math.SinF(0.0f);

for explicitly using floats only. This would clearly become an API addition then however.

@colombod
Copy link
Member

If there was a way to express generic code with "numeric" bounds for the type parameters......

Dr Diego Colombo PhD

On 21 Jan 2015, at 08:17, Jordan Singh notifications@github.com wrote:

@mikedn Never even thought about seeing how integers would work in a scenario like this. Your right, this change would disallow the possibility of doubles from being returned if using an int. As an alternative could be a new API e.g.

Math.SinF(0.0f);
for explicitly using floats only. This would clearly become an API addition then however.


Reply to this email directly or view it on GitHub.

@mikedn
Copy link
Contributor

mikedn commented Jan 21, 2015

@xanather "Disallow" is a bit too strong, you can still call the double overload by explicitly casting to double, as in f((double)int.MaxValue). The main problem is that existing code can be affected by the overload addition if doesn't have such a cast already.

I must say that I'm baffled by C#'s choice of overload in this case. Out of 3 possibilities it picks the worst, the one which leads to data loss. Picking the double overload would be a better choice as it avoids data loss. Rejecting the code with an "ambiguous overload" error would also be a good choice (C++ does this). But picking the float overload, hmm...

@terrajobst
Copy link
Member

@ellismg, you mentioned that there is already some work in progress that might cover aspects of this. Can you link to that work?

@ellismg
Copy link
Contributor

ellismg commented Sep 11, 2015

dotnet/coreclr#710

@joshfree joshfree assigned mellinoe and unassigned terrajobst Dec 2, 2015
@tannergooding
Copy link
Member

dotnet/coreclr#5492 is tracking the remaining work here. The CoreCLR side changes have been merged already.

@mellinoe
Copy link
Contributor

mellinoe commented Nov 8, 2016

We've added methods that cover all of the functionality that was exposed for double. Please file a new issue if you feel there is more to be done.

@mellinoe mellinoe closed this as completed Nov 8, 2016
Olafski referenced this issue in Olafski/corefx Jun 15, 2017
Currently the link for 14.04 downloads the 16.04 version and vice-versa.
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
@dotnet dotnet locked as resolved and limited conversation to collaborators Jan 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Runtime
Projects
None yet
Development

No branches or pull requests

9 participants