Math.Round and misunderstanding #38160

FrantzUml · 2020-06-19T18:17:35Z

Math.Round with rounding type

The MidpointRounding enum is a "rounding to nearest integer" and not a "directed rounding".
So, for example, the MidpointRounding.ToZero description says : "when a number is halfway between two others......."
So, System.Math.Round(-1.8D, any type of MidpointRounding) should return -2 because it's the nearest integer value of -1.8.

But, with ToPositiveInfinity and ToZero, the result is badly -1

It's a misunderstanding of the rounding math theory.

The result should be different only when the value to round is halfway between two others.

Watching the code for Core 3.1, the developpers make select between the MidpointRounding mode even when the value to round is not halfway.

In definitive, the System.Math is buggy.

Cordially

ghost · 2020-06-19T18:17:39Z

Tagging subscribers to this area: @tannergooding
Notify danmosemsft if you want to be subscribed.

SingleAccretion · 2020-06-19T18:34:46Z

Even if unintuitive (which I agree it is, and the documentation does not help either), this behavior cannot be changed as it would be the worst kind of a breaking change (silent change of runtime behavior). The bar for these changes is incredibly high.

FrantzUml · 2020-06-19T18:47:19Z

You're right with the breaking change, but the documentation msdn says :
the MidpointRounding enumeration does not affect the result of the rounding operation when the next digit is from 0 to 4 and from 6 to 9.
However, if the next digit is 5, which is the midpoint between two possible results, and all remaining digits are zero or there are no remaining digits, the nearest number is ambiguous. In this case, the MidpointRounding enumeration enables you to specify whether the rounding operation returns the nearest number away from zero or the nearest even number.

So, the MidpointRounding mode is quiet a "nearest rounding" and not a "directed rounding".
For example, the documentation of msdn about ToPositiveInfinity says "When a number is halfway between two others, it is rounded toward the result closest to and no less than the infinitely precise result."
But he comments in the source code indicates "Directed rounding: Round up to the next value, toward positive infinity".

Is it a bad design and conception from start ?

SingleAccretion · 2020-06-19T19:04:39Z

Is it a bad design and conception from start?

I would say it is a confusing design (the name of the enum is indeed MidpointRounding). But given it is a very recent addition to the framework (see here), I suspect the reasoning behind it is discoverability, which was determined to be of higher value than consistency (see the original issue, I suppose). I might be able to dig up an API review for it from YouTube.

However, what seems quite clear is that docs need some improvement. Maybe file a doc bug (or, if you want, create a pull request)?

tannergooding · 2020-06-19T19:10:41Z

The APIs were originally added to cover some missing rounding modes from the IEEE 754 specification.

I don't recall the exact reasoning why we decided to add them to MidpointRounding rather than some new DirectedRounding type.

At a minimum the docs should be updated to call out this difference. Fixing it would be a breaking change and would require further consideration.

The algorithm for Math.Round is notably already buggy, even for the rounding modes that existed in .NET Framework and #1643 tracks fixing it. This is also a breaking change.

SingleAccretion · 2020-06-19T19:31:19Z

Actually, the more I think about these APIs, the more confusing they seem to me (AwayFromZero and ToZero aren't actually related, heh?). I am not a smart person, but even accounting for this, it does take me an unusually high amount of mental effort to understand what "When a number is halfway between two others, it is rounded toward the result closest to and no greater in magnitude than the infinitely precise result" is supposed to mean (this, I guess, can be fixed with better documentation, but still).

At this point, I would be in favor of obsoleting these enum members (and EB-nevering them, as there would presumably be a 1-1 replacement, but I know this has drawbacks and so people aren't enthusiastic about it) and actually adding a dedicated enum. This would only be source-breaking, so it might actually be actionable (and, presumably, not many people are actually using these APIs yet so the impact would be limited).

@FrantzUml Do you feel strongly enough about this to edit the original issue into a dedicated obsoletion proposal or open another one and close this one? You would have my vote.

Edit: the API review.
Some quotes (slightly altered as to suit the professional setting of this issue):

"Yes, people will not be happy", "All the people that are trying to do rounding and nobody knows what these things mean. Like, it's not super hard, but it requires a decent amount of understanding on what is going on. It's a typical problem of rounding not doing what you want it to do..."
"It will need a good documentation. That's for sure"

The point of it not really being a "midpoint rounding" was brought up, but nobody had strong opinions about the alternatives - it seems like the consensus was that it is fairly advanced API and you will have to read the docs anyway, so it was approved.

FrantzUml · 2020-06-22T17:41:49Z

I agree with no breaking change but adding a dedicated enum, explain the bug in MidPointRounding and make it obsolete.
So, the enum should be :
public enum RoundingMode
{
///

/// Directed round down (same as Floor).
///

///
/// Assert.AreEqual(-2D, -1.8D);
/// Assert.AreEqual(-2D, -1.5D);
/// Assert.AreEqual(-2D, -1.2D);
/// Assert.AreEqual(1D, +1.2D);
/// Assert.AreEqual(1D, +1.5D);
/// Assert.AreEqual(1D, +1.8D);
///
Down,

/// <summary>
/// Directed round up (same as Ceiling).
/// </summary>
/// <remarks>
/// <code>Assert.AreEqual(-1D, -1.8D);</code>
/// <code>Assert.AreEqual(-1D, -1.5D);</code>
/// <code>Assert.AreEqual(-1D, -1.2D);</code>
/// <code>Assert.AreEqual(2D, +1.2D);</code>
/// <code>Assert.AreEqual(2D, +1.5D);</code>
/// <code>Assert.AreEqual(2D, +1.8D);</code>
/// </remarks>
Up,

/// <summary>
/// Directed round towards zero (same as Truncate).
/// </summary>
/// <remarks>
/// <code>Assert.AreEqual(-1D, -1.8D);</code>
/// <code>Assert.AreEqual(-1D, -1.5D);</code>
/// <code>Assert.AreEqual(-1D, -1.2D);</code>
/// <code>Assert.AreEqual(1D, +1.2D);</code>
/// <code>Assert.AreEqual(1D, +1.5D);</code>
/// <code>Assert.AreEqual(1D, +1.8D);</code>
/// </remarks>
TowardsZero,

/// <summary>
/// Directed round away from zero
/// </summary>
/// <remarks>
/// <code>Assert.AreEqual(-2D, -1.8D);</code>
/// <code>Assert.AreEqual(-2D, -1.5D);</code>
/// <code>Assert.AreEqual(-2D, -1.2D);</code>
/// <code>Assert.AreEqual(2D, +1.2D);</code>
/// <code>Assert.AreEqual(2D, +1.5D);</code>
/// <code>Assert.AreEqual(2D, +1.8D);</code>
/// </remarks>
AwayZero,

/// <summary>
/// Round to nearest integer and down when the number is halfway between two others.
/// </summary>
/// <remarks>
/// <code>Assert.AreEqual(-2D, -1.8D);</code>
/// <code>Assert.AreEqual(-2D, -1.5D);</code>
/// <code>Assert.AreEqual(-1D, -1.2D);</code>
/// <code>Assert.AreEqual(1D, +1.2D);</code>
/// <code>Assert.AreEqual(1D, +1.5D);</code>
/// <code>Assert.AreEqual(2D, +1.8D);</code>
/// </remarks>
HalfDown,

/// <summary>
/// Round to nearest integer and up when the number is halfway between two others.
/// </summary>
/// <remarks>
/// <code>Assert.AreEqual(-2D, -1.8D);</code>
/// <code>Assert.AreEqual(-1D, -1.5D);</code>
/// <code>Assert.AreEqual(-1D, -1.2D);</code>
/// <code>Assert.AreEqual(1D, +1.2D);</code>
/// <code>Assert.AreEqual(2D, +1.5D);</code>
/// <code>Assert.AreEqual(2D, +1.8D);</code>
/// </remarks>
HalfUp,

/// <summary>
/// Round to nearest integer and towards zero when the number is halfway between two others.
/// </summary>
/// <remarks>
/// <code>Assert.AreEqual(-2D, -1.8D);</code>
/// <code>Assert.AreEqual(-1D, -1.5D);</code>
/// <code>Assert.AreEqual(-1D, -1.2D);</code>
/// <code>Assert.AreEqual(1D, +1.2D);</code>
/// <code>Assert.AreEqual(1D, +1.5D);</code>
/// <code>Assert.AreEqual(2D, +1.8D);</code>
/// </remarks>
HalfTowardsZero,

/// <summary>
/// Round to nearest integer and away from zero when the number is halfway between two others.
/// </summary>
/// <remarks>
/// <code>Assert.AreEqual(-2D, -1.8D);</code>
/// <code>Assert.AreEqual(-2D, -1.5D);</code>
/// <code>Assert.AreEqual(-1D, -1.2D);</code>
/// <code>Assert.AreEqual(1D, +1.2D);</code>
/// <code>Assert.AreEqual(2D, +1.5D);</code>
/// <code>Assert.AreEqual(2D, +1.8D);</code>
/// </remarks>
HalfAwayZero,

/// <summary>
/// Round to nearest integer and to nearest even integer when the number is halfway between two others.
/// </summary>
/// <remarks>
/// <code>Assert.AreEqual(-2D, -1.8D);</code>
/// <code>Assert.AreEqual(-2D, -1.5D);</code>
/// <code>Assert.AreEqual(-1D, -1.2D);</code>
/// <code>Assert.AreEqual(0D, -0.5D);</code>
/// <code>Assert.AreEqual(0D, +0.5D);</code>
/// <code>Assert.AreEqual(1D, +1.2D);</code>
/// <code>Assert.AreEqual(2D, +1.5D);</code>
/// <code>Assert.AreEqual(2D, +1.8D);</code>
/// </remarks>
HalfToEven

}

And the proposed implementation :
public static double Round(double value, RoundingMode rounding)
{
if (rounding == RoundingMode.Down)
{
return System.Math.Floor(value);
}
else if (rounding == RoundingMode.Up)
{
return System.Math.Ceiling(value);
}
else if (rounding == RoundingMode.TowardsZero)
{
return System.Math.Truncate(value);
}
else if (rounding == RoundingMode.AwayZero)
{
return System.Math.Sign(value) < 0 ? System.Math.Floor(value) : System.Math.Ceiling(value);
}
else if (rounding == RoundingMode.HalfToEven)
{
return System.Math.Round(value);
}
else if (rounding == RoundingMode.HalfDown)
{
return System.Math.Ceiling(value - 0.5D);
}
else if (rounding == RoundingMode.HalfUp)
{
return System.Math.Floor(value + 0.5D);
}
else if (rounding == RoundingMode.HalfTowardsZero)
{
double absolute = System.Math.Ceiling(System.Math.Abs(value) - 0.5D);
return (System.Math.Sign(value) > 0) ? absolute : -absolute;
}
else if (rounding == RoundingMode.HalfAwayZero)
{
double absolute = System.Math.Floor(System.Math.Abs(value) + 0.5D);
return (System.Math.Sign(value) > 0) ? absolute : -absolute;
}
else
{
throw new ArgumentException();
}
}

Or with using the static ModF function (but i can't verify compliance as the function is internal in Core) :
public static unsafe double Round(double value, RoundingMode rounding)
{
if (rounding == RoundingMode.Down)
{
return System.Math.Floor(value);
}
else if (rounding == RoundingMode.Up)
{
return System.Math.Ceiling(value);
}
else if (rounding == RoundingMode.TowardsZero)
{
return System.Math.Truncate(value);
}
else if (rounding == RoundingMode.AwayZero)
{
return System.Math.Sign(value) < 0 ? System.Math.Floor(value) : System.Math.Ceiling(value);
}
else if (rounding == RoundingMode.HalfToEven)
{
return System.Math.Round(value);
}
else
{
double fraction = ModF(value, &value);
double absoluteFrac = System.Math.Abs(fraction);
if (absoluteFrac < 0.5D)
{
return value;
}
int signFrac = System.Math.Sign(fraction);
switch (rounding)
{
case RoundingMode.HalfAwayZero:
return value + signFrac;
case RoundingMode.HalfDown:
return (signFrac < 0) ? value + signFrac : (absoluteFrac > 0.5D) ? value + signFrac : value;
case RoundingMode.HalfUp:
return (signFrac > 0) ? value + signFrac : (absoluteFrac > 0.5D) ? value + signFrac : value;
case RoundingMode.HalfTowardsZero:
return (absoluteFrac > 0.5D) ? value + signFrac : value;
default:
throw new ArgumentException();
}
}
}

I will suggest implementation with "int digits" as argument, but I don't like the implmentation in Core because multiply value by 10^digits may overflow or create less accuracy.

SingleAccretion · 2020-06-22T20:00:48Z

@FrantzUml I think we need to support rounding to the specified number of significant digits (Math.Round(double value, int digits, MidpointRounding mode) overload), not just to digits.

I would open a new issue for the proposal and close this one (to avoid modifying your original post).

FrantzUml · 2020-06-24T18:24:14Z

I suggest this implementation.
Do you think I have to pull a request and close this issue ? (sorry but I'm not a professional developer and not very used to github).

public static unsafe double Round(double value, int digits, RoundingMode rounding) 
{ 
	if ((digits < 0) || (digits > kMaxDoubleDigits))
	{
		throw new ArgumentException();
	}
	if (digits == 0)
	{
		return Round(value, rounding);
	}
	double fraction = ModF(value, &value);
	double power10 = roundPower10Double[digits];
	fraction *= power10;
	return value + Round(fraction, rounding) / power10;
}

public static unsafe double Round(double value, RoundingMode rounding)
{
	if (rounding < RoundingMode.HalfToEven || rounding > RoundingMode.HalfTowardsZero)
	{
		throw new ArgumentException();
	}
	if (System.Math.Abs(value) < kRoundDoubleLimit)
	{
		if (rounding == RoundingMode.Down)
		{
			return System.Math.Floor(value);
		}
		else if (rounding == RoundingMode.Up)
		{
			return System.Math.Ceiling(value);
		}
		else if (rounding == RoundingMode.TowardsZero)
		{
			return System.Math.Truncate(value);
		}
		else if (rounding == RoundingMode.AwayZero)
		{
			return System.Math.Sign(value) < 0 ? System.Math.Floor(value) : System.Math.Ceiling(value);
		}
		else if (rounding == RoundingMode.HalfToEven)
		{
			return System.Math.Round(value);
		}
		else
		{
			double fraction = ModF(value, &value);
			double absoluteFrac = System.Math.Abs(fraction);
			if (absoluteFrac < 0.5D)
			{
				return value;
			}
			int signFrac = System.Math.Sign(fraction);
			switch (rounding)
			{
				case RoundingMode.HalfAwayZero:
					return value + signFrac;
				case RoundingMode.HalfDown:
					return (signFrac < 0) ? value + signFrac : (absoluteFrac > 0.5D) ? value + signFrac : value;
				case RoundingMode.HalfUp:
					return (signFrac > 0) ? value + signFrac : (absoluteFrac > 0.5D) ? value + signFrac : value;
				case RoundingMode.HalfTowardsZero:
					return (absoluteFrac > 0.5D) ? value + signFrac : value;
				default:
					throw new ArgumentException();
			}
		}
	}
	return value;
}

`
public enum RoundingMode
{
	/// <summary>`
	`/// Directed round down (same as Floor).`
	`/// </summary>`
	`/// <remarks>`
	`/// <code>Assert.AreEqual(-2D, -1.8D);</code>`
	`/// <code>Assert.AreEqual(-2D, -1.5D);</code>`
	`/// <code>Assert.AreEqual(-2D, -1.2D);</code>`
	`/// <code>Assert.AreEqual(1D, +1.2D);</code>`
	`/// <code>Assert.AreEqual(1D, +1.5D);</code>`
	`/// <code>Assert.AreEqual(1D, +1.8D);</code>`
	`/// </remarks>`
	Down = 3,

	/// <summary>
	/// Directed round up (same as Ceiling).
	/// </summary>
	/// <remarks>
	/// <code>Assert.AreEqual(-1D, -1.8D);</code>
	/// <code>Assert.AreEqual(-1D, -1.5D);</code>
	/// <code>Assert.AreEqual(-1D, -1.2D);</code>
	/// <code>Assert.AreEqual(2D, +1.2D);</code>
	/// <code>Assert.AreEqual(2D, +1.5D);</code>
	/// <code>Assert.AreEqual(2D, +1.8D);</code>
	/// </remarks>
	Up = 4,

	/// <summary>
	/// Directed round towards zero (same as Truncate).
	/// </summary>
	/// <remarks>
	/// <code>Assert.AreEqual(-1D, -1.8D);</code>
	/// <code>Assert.AreEqual(-1D, -1.5D);</code>
	/// <code>Assert.AreEqual(-1D, -1.2D);</code>
	/// <code>Assert.AreEqual(1D, +1.2D);</code>
	/// <code>Assert.AreEqual(1D, +1.5D);</code>
	/// <code>Assert.AreEqual(1D, +1.8D);</code>
	/// </remarks>
	TowardsZero = 2,

	/// <summary>
	/// Directed round away from zero
	/// </summary>
	/// <remarks>
	/// <code>Assert.AreEqual(-2D, -1.8D);</code>
	/// <code>Assert.AreEqual(-2D, -1.5D);</code>
	/// <code>Assert.AreEqual(-2D, -1.2D);</code>
	/// <code>Assert.AreEqual(2D, +1.2D);</code>
	/// <code>Assert.AreEqual(2D, +1.5D);</code>
	/// <code>Assert.AreEqual(2D, +1.8D);</code>
	/// </remarks>
	AwayZero = 5,

	/// <summary>
	/// Round to nearest integer and down when the number is halfway between two others.
	/// </summary>
	/// <remarks>
	/// <code>Assert.AreEqual(-2D, -1.8D);</code>
	/// <code>Assert.AreEqual(-2D, -1.5D);</code>
	/// <code>Assert.AreEqual(-1D, -1.2D);</code>
	/// <code>Assert.AreEqual(1D, +1.2D);</code>
	/// <code>Assert.AreEqual(1D, +1.5D);</code>
	/// <code>Assert.AreEqual(2D, +1.8D);</code>
	/// </remarks>
	HalfDown = 6,

	/// <summary>
	/// Round to nearest integer and up when the number is halfway between two others.
	/// </summary>
	/// <remarks>
	/// <code>Assert.AreEqual(-2D, -1.8D);</code>
	/// <code>Assert.AreEqual(-1D, -1.5D);</code>
	/// <code>Assert.AreEqual(-1D, -1.2D);</code>
	/// <code>Assert.AreEqual(1D, +1.2D);</code>
	/// <code>Assert.AreEqual(2D, +1.5D);</code>
	/// <code>Assert.AreEqual(2D, +1.8D);</code>
	/// </remarks>
	HalfUp = 7,

	/// <summary>
	/// Round to nearest integer and towards zero when the number is halfway between two others.
	/// </summary>
	/// <remarks>
	/// <code>Assert.AreEqual(-2D, -1.8D);</code>
	/// <code>Assert.AreEqual(-1D, -1.5D);</code>
	/// <code>Assert.AreEqual(-1D, -1.2D);</code>
	/// <code>Assert.AreEqual(1D, +1.2D);</code>
	/// <code>Assert.AreEqual(1D, +1.5D);</code>
	/// <code>Assert.AreEqual(2D, +1.8D);</code>
	/// </remarks>
	HalfTowardsZero = 8,

	/// <summary>
	/// Round to nearest integer and away from zero when the number is halfway between two others.
	/// </summary>
	/// <remarks>
	/// <code>Assert.AreEqual(-2D, -1.8D);</code>
	/// <code>Assert.AreEqual(-2D, -1.5D);</code>
	/// <code>Assert.AreEqual(-1D, -1.2D);</code>
	/// <code>Assert.AreEqual(1D, +1.2D);</code>
	/// <code>Assert.AreEqual(2D, +1.5D);</code>
	/// <code>Assert.AreEqual(2D, +1.8D);</code>
	/// </remarks>
	HalfAwayZero = 1,

	/// <summary>
	/// Round to nearest integer and to nearest even integer when the number is halfway between two others.
	/// </summary>
	/// <remarks>
	/// <code>Assert.AreEqual(-2D, -1.8D);</code>
	/// <code>Assert.AreEqual(-2D, -1.5D);</code>
	/// <code>Assert.AreEqual(-1D, -1.2D);</code>
	/// <code>Assert.AreEqual(0D, -0.5D);</code>
	/// <code>Assert.AreEqual(0D, +0.5D);</code>
	/// <code>Assert.AreEqual(1D, +1.2D);</code>
	/// <code>Assert.AreEqual(2D, +1.5D);</code>
	/// <code>Assert.AreEqual(2D, +1.8D);</code>
	/// </remarks>
	HalfToEven = 0
}

SingleAccretion · 2020-06-24T20:43:35Z

Do you think I have to pull a request and close this issue ? (sorry but I'm not a professional developer and not very used to github).

So, this is the process for API changes like this one in the runtime repository:

1. Someone creates an API proposal, that has:

The motivation behind the proposal.
The proposed APIs.
Other relevant info.

#13933 is often quoted as a good template for a proposal.

2. Someone from the team thinks it would be worth implementing and marks the proposal api-ready-for-review.

3. The proposal gets reviewed (you can watch reviews live on .NET Foundation's YouTube channel). When this happens depends on what status did the proposal get:

Blocking: the highest priority, major features planned for a release depend on the implementation of the API.
Backlog: everything else. Usually, these are "nice to have" APIs, but can sometimes be bigger work items. Anything not marked as Blocking is automatically put on the backlog.

The team reviews backlog in a chronological order (so, older issues get reviewed first), except when a dedicated review session (or many of them) is needed for something big (recent examples: #34742, #1793). You can see issues that are about to get reviewed via http://aka.ms/ready-for-api-review)

4. The team can decide that the proposal:

Is approved. Most proposals end up being approved, mostly because there had to be a reason someone marked it as api-ready-for-review in the first place.
Needs work. Sometimes the API shape is too rough and authors of the proposal must improve it before it can be reviewed again. This is also sometimes used for bigger items that need many dedicated review sessions to be completed.
Is rejected. Although rare, some proposals are not approved, for various reasons (backwards compatibility and lack of value are the major ones).

5. After approval, the proposal must be implemented (if it hasn't been already). Easier issues can be marked as up-for-grabs, which means that the team accepts pull requests with implementations for those from the community.

In view of all this:

There are two proposals in this issue, with different motivations:
- Obsolete (and hide) some members on MidpointRounding and replace them with a new enum (DirectedRounding), to clear up the confusion between, for example, AwayFromZero and ToZero.
- Add new rounding modes to support more scenarios (your later posts).

I would suggest you create two new issues for each, formatted as proper proposals, with a link to this issue. I would also highly encourage you to create an issue in the docs repo (https://github.com/dotnet/docs) with a request to improve the documentation on the new rounding modes. You can improve the docs yourself by creating a PR there, if you feel that you can add the missing bits (I would do it myself, but unfortunately I do not have the time for it right now).

danmoseley · 2020-06-24T20:47:51Z

@SingleAccretion that is a great summary I wonder whether it would be worth a PR to improve https://github.com/dotnet/runtime/blob/master/docs/project/api-review-process.md? Also note we now have a template: https://github.com/dotnet/runtime/issues/new?assignees=&labels=api-suggestion&template=02_api_proposal.md&title=

@FrantzUml you can format code nicely by using three back ticks. So you start with

```c#

on its own line and end with 3 ticks on their own line. I updated your post above as an example.

SingleAccretion · 2020-06-24T21:06:21Z

@danmosemsft I think the document is great, the only things I would add are these links:

https://aka.ms/ready-for-api-review
.NET Foundation's YouTube channel
https://apisof.net

Edit: and a link to the template, heh.

I am not quite sure yet how to properly fit these into the structure (inline/separate section) and it is approaching midnight here so I'll leave it to my tomorrow's self to figure out.

wischi-chr · 2022-09-01T13:16:22Z

Also just ran into the problem that ToZero isn't a mid-point rounding strategy.

For what it's worth IMHO an API design like the following would handle all cases and would also be perfectly clear for the developers and consistent.

public enum RoundingDirection
{
    ToEven,
    ToOdd,
    ToZero,
    AwayFromZero,
    ToPositiveInfinity,
    ToNegativeInfinity,
}

public enum RoundingStrategy
{
    ToNearest,
    Directed,
}

public static class Math
{
    public static double Round
    (
        double value,
        int digits,
        RoundindDirection direction = RoundindDirection.ToEven,
        RoundingStrategy strategy = RoundingStrategy.ToNearest
    )
    {
        // implementation left as an exercise for the reader
        throw new NotImplementedException();
    }
}

Of course there would be combinations that are probably very rare (like ToEven + Directed) but at least it would be consistant. This would also solve #40456

Grimeh · 2023-01-05T11:50:17Z

Today I ran into the issue of MidpointRounding being a confusing name when trying to determine if there was a way to do a directed AwayFromZero round in the Math API. The improved docs helped a lot, particularly the remarks section for MidpointRounding. I was disappointed to discover there was a directed ToZero but no matching directed AwayFromZero.

I like @SingleAccretion's idea of:

introducing a new DirectedRounding enum
adding overloads to Round for Math, MathF, and Decimal: Round([numeric type], DirectedRounding) and Round([numeric type], int, DirectedRounding)
marking MidpointRounding.ToNegativeInfinity, MidpointRounding.ToPositiveInfinity, and MidpointRounding.ToZero as obsolete in favour of their DirectedRounding counterparts (though I'm ignorant as to what "EB-nevering" means, sounds like a way to hide it?)

An issue I can see is that having a non-directed actually-midpoint MidpointRounding.ToZero is helpful if we wish to fill out the API for rounding modes, but changing the behaviour of the existing enum value would be undesirable for obvious reasons. Marking the existing ToZero value obsolete and adding a MidpointRounding.TowardsZero may fix that, but the ambiguity/similarity with MidpointRounding.ToZero might be confusing if the user who has used ToZero previously.

It also adds two more overloads to a method that already has a few, not sure if that's a concern or not.

I'm happy to make a proposal, should I bring up the above concerns in a new API proposal issue under the "Risks" section (as detailed here), or discuss them here before making a new issue?
Please forgive my ignorance, first time contributor :)

Corniel · 2024-07-04T07:11:34Z

For a open source project, I maintain, I once implemented an extended version of rounding (only for decimals, but that should not matter).

I extended the support methods of decimal rounding to 13 (see list below), and also allow a negative amount of decimals to round to, to be specified, meaning to round to a multiple of 10, 100, etc...

Might that help fixing this API?

/// <summary>Methods of rounding <see cref="decimal"/>s.</summary>
/// <remarks>
/// This is an extension on <see cref="MidpointRounding"/>.
/// </remarks>
public enum DecimalRounding
{
    /// <summary>When a number is halfway between two others, it is rounded toward the nearest even.</summary>
    ToEven = 0,

    /// <summary>Bankers round, also known as <see cref="ToEven"/>.</summary>
    BankersRound = ToEven,

    /// <summary>When a number is halfway between two others, it is rounded toward the nearest number that is away from zero.</summary>
    AwayFromZero = 1,

    /// <summary>When a number is halfway between two others, it is rounded toward the nearest odd.</summary>
    ToOdd = 2,

    /// <summary>When a number is halfway between two others, it is rounded toward the nearest number that is closest to zero.</summary>
    TowardsZero = 3,

    /// <summary>When a number is halfway between two others, it is rounded toward the highest of the two.</summary>
    Up = 4,

    /// <summary>When a number is halfway between two others, it is rounded toward the lowest of the two.</summary>
    Down = 5,

    /// <summary>When a number is halfway between two others, it is randomly rounded up or down with equal probability.</summary>
    RandomTieBreaking = 6,

    /// <summary>When a number is between two others, the remainder is truncated/ignored.</summary>
    Truncate = 7,

    /// <summary>When a number is between two others, it is rounded toward the nearest number that is away from zero.</summary>
    DirectAwayFromZero = 8,

    /// <summary>When a number is between two others, it is rounded toward the nearest number that is closest to zero.</summary>
    DirectTowardsZero = 9,

    /// <summary>When a number is between two others, its rounded to the largest.</summary>
    Ceiling = 10,

    /// <summary>When a number is between two others, its rounded to the smallest.</summary>
    Floor = 11,

    /// <summary>When a number is between two others, it is randomly rounded up or down with stochastic probability.</summary>
    StochasticRounding = 12,
}

Source

Dotnet-GitSync-Bot added area-System.Numerics untriaged New issue has not been triaged by the area owner labels Jun 19, 2020

tannergooding added this to the Future milestone Jun 23, 2020

SingleAccretion mentioned this issue Jun 25, 2020

Imrovements to api-review-process.md #38378

Merged

FrantzUml mentioned this issue Jun 29, 2020

Issue 38160 - MidpointRounding is buggy #38563

Closed

scalablecory added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jun 29, 2020

SingleAccretion mentioned this issue Jul 4, 2020

Improve System.MidpointRounding documentation dotnet/docs#19314

Closed

tannergooding removed the untriaged New issue has not been triaged by the area owner label Jul 6, 2020

SingleAccretion mentioned this issue Aug 6, 2020

[Proposal] Add new overload to Math.Truncate #40456

Open

sWW26 mentioned this issue Mar 15, 2021

Add support for MidpointRounding on Math.Round fable-compiler/Fable#2389

Closed

raverx76 mentioned this issue Jun 4, 2021

rounding raverx76/SalsBreakdown#2

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Math.Round and misunderstanding #38160

Math.Round and misunderstanding #38160

FrantzUml commented Jun 19, 2020

ghost commented Jun 19, 2020

SingleAccretion commented Jun 19, 2020

FrantzUml commented Jun 19, 2020

SingleAccretion commented Jun 19, 2020

tannergooding commented Jun 19, 2020

SingleAccretion commented Jun 19, 2020 •

edited

Loading

FrantzUml commented Jun 22, 2020

SingleAccretion commented Jun 22, 2020

FrantzUml commented Jun 24, 2020 •

edited by danmoseley

Loading

SingleAccretion commented Jun 24, 2020 •

edited

Loading

danmoseley commented Jun 24, 2020 •

edited

Loading

SingleAccretion commented Jun 24, 2020 •

edited

Loading

wischi-chr commented Sep 1, 2022 •

edited

Loading

Grimeh commented Jan 5, 2023

Corniel commented Jul 4, 2024

Math.Round and misunderstanding #38160

Math.Round and misunderstanding #38160

Comments

FrantzUml commented Jun 19, 2020

Math.Round with rounding type

ghost commented Jun 19, 2020

SingleAccretion commented Jun 19, 2020

FrantzUml commented Jun 19, 2020

SingleAccretion commented Jun 19, 2020

tannergooding commented Jun 19, 2020

SingleAccretion commented Jun 19, 2020 • edited Loading

FrantzUml commented Jun 22, 2020

SingleAccretion commented Jun 22, 2020

FrantzUml commented Jun 24, 2020 • edited by danmoseley Loading

SingleAccretion commented Jun 24, 2020 • edited Loading

danmoseley commented Jun 24, 2020 • edited Loading

SingleAccretion commented Jun 24, 2020 • edited Loading

wischi-chr commented Sep 1, 2022 • edited Loading

Grimeh commented Jan 5, 2023

Corniel commented Jul 4, 2024

SingleAccretion commented Jun 19, 2020 •

edited

Loading

FrantzUml commented Jun 24, 2020 •

edited by danmoseley

Loading

SingleAccretion commented Jun 24, 2020 •

edited

Loading

danmoseley commented Jun 24, 2020 •

edited

Loading

SingleAccretion commented Jun 24, 2020 •

edited

Loading

wischi-chr commented Sep 1, 2022 •

edited

Loading