round() macro in Arduino.h

[Koepel]

The function round() in "math.h" is okay, it returns a floating point number as expected.
The macro round() in "Arduino.h" is a bug. It can not handle large floating point numbers and it returns a long integer.
#define round(x) ((x)>=0?(long)((x)+0.5):(long)((x)-0.5))
It could be renamed to something else, for example ROUND_INT().

The sketch below shows the difference. With the normal round() function it returns four times "1.50", and with the round() macro in "Arduino.h" it returns: "1.00", "1.50", "1.50", "0.00".

// Arduino IDE 1.81 with Arduino Uno

// The "#undef round" removes the round() macro in Arduino.h
// Try with and without the #undef round
// #undef round

void setup() 
{
  float x;
  Serial.begin( 9600);

  x = 15.0;
  x = round( x) / 10;   // a integer division with the macro
  Serial.println( x);   // wrong value with the macro

  x = 15.0;
  x = round( x) / 10.0; // always calculated with floats
  Serial.println( x);   // always okay

  x = 15.0;
  x = round( x);
  x /= 10;              // always calculated with floats
  Serial.println( x);   // always okay

  x = 1.5E+20;
  x = round( x);        // the macro fails
  x /= 1E+20;           // make it printable with Serial.println
  Serial.println( x);   // wrong value with macro 
}

void loop()
{
}

[facchinm]

In fact, round in the Arduino world returns the nearest integer for the argument (this also happen with math.h round, see http://www.codecogs.com/library/computing/c/math.h/round.php).
None of the examples you are giving is wrong:

• x = round( x) / 10; is a division between integers, the result will be an integer (1) promoted to a float (1.00)
• x = round( x) / 10.0; is a division between an integer and a float, so the int gets promoted to a float and the result is a float (1.50)
• x = 1.5E+20; is bigger than a long (uint32_t) in AVR world, so in is expected to fail in funny ways

I'd close it as wontfix any thoughts about that @cmaglie @matthijskooijman ?

[Koepel]

It violates the IEEE standard, making the Arduino floating point unpredictable. It is really a terrible bug.
The round() function is supposed to return a float. The x = round( x) / 10; should be a floating point calculation.
Are you saying that a Arduino single precision floating point may not be larger than the value of a long ?

[facchinm]

It should be a floating point calculation if round(x) would return a float, but since it returns a long it's an integer division.
However, you are totally right; I didn't want to defend the macro implementation, which is terribly error prone, but only state that adopting math.h round could have some side effects on older sketches.
I just tested the sketch size occupation and, with my biggest disbelief, the math.h version produces way smaller binaries than the macro version

Arduino.h round:

Sketch uses 3034 bytes (9%) of program storage space. Maximum is 32256 bytes.
Global variables use 198 bytes (9%) of dynamic memory, leaving 1850 bytes for local variables. Maximum is 2048 bytes.

math.h round

Sketch uses 1742 bytes (5%) of program storage space. Maximum is 32256 bytes.
Global variables use 186 bytes (9%) of dynamic memory, leaving 1862 bytes for local variables. Maximum is 2048 bytes.

Some investigation is surely needed at this point, thanks for pointing it out and sorry for the previous response

[Koepel]

The round() function should return a floating point number. Some explanations for other platforms say that it returns the nearest integer, that might be confusing, but those functions still return a floating point.
I don't care about the macro or older sketches or smaller code. The round() function should be reliable and predictable. The round() function in "math.h" is just that, as it should be. That is however ruined by the macro.

[cousteaulecommandant]

The thing is, Arduino is not standard C++, but some sort of language of its own that is based on C++ but introduces some simplifications. Among other things, it creates its own functions (well, macros) for round, min, max, abs, etc.
If you don't like the behavior of this "Arduino language" and want to use actual C++, create a new tab in your project and name it main.cpp (or if you prefer C, main.c), put your code there and leave the main tab empty. You'll be programming in pure C++ and not get the dreaded Arduino.h.

Alternatively, if you want to use math.h's round() within the Arduino language because you also want to use the rest of Arduino features (e.g. digitalWrite()), you can use (round)(x) so that round is not interpreted as a macro, or simply write #undef round at the beginning of the .ino file, which I think should do the trick (untested).

Now, some thoughts:

• Should Arduino have used different names (e.g. Round, Abs) rather than preexisting ones? Probably, but I guess it's too late to change that now.
• Is it really a problem to use a name already defined in C++? I'd say it's not as long as users are aware that they're not using pure C++ but something built on top of it.
• Should this odd behavior on overflow be documented? Definitely, but there doesn't even seem to be documentation for Arduino's round().
• Is it a good idea to use macros for this? Honestly I don't think so; I'd rather use templated functions. Otherwise stuff like round(x++) will do weird stuff. (Then again, this problem is documented for min and max.)
• Is there any advantage on using this implementation over just using the standard round()? Well I would have thought so, but then @facchinm and his test confused me. (Are you sure the call to round() is not being optimized out or something? Maybe if you call round() with a constant argument the compiler is smart enough to optimize it.)

[Koepel]

@cousteaulecommandant, I have to disagree with you. There is no good reason to change things for the bad. If you are right, then let's change 'sin', 'tan' and 'memset' as well, just for the fun of it. There has been put so much effort, for example, into the avr libc math library, and it is a very reliable and fast. Why change something that is perfectly working well ?

Meanwhile I learned that the Java round function returns an integer instead of a floating point number of the nearest whole number. Most people seem to think of that as a mistake when the Java language was created.

Since Arduino uses 'c' and 'c++' the round function should work as expected. In my opinion it really ruins the math library and it is not a simplification and it has not to do with being pure or not.

[PaulStoffregen]

Why change something that is perfectly working well ?

Perhaps you are unaware of Arduino's long 12 year history? Not everything you take for granted as working perfectly well in the modern versions of avr-libc was always so reliable.

[cousteaulecommandant]

Not everything you take for granted as working perfectly well in the modern versions of avr-libc was always so reliable.

In fact, round was not even a standard C++ function until C++11, so Arduino is not at fault for defining a function that did not exist yet. Plus C++ was quite successful even prior to C++11, so I would not say that not being able to use round "really ruins the math library".

(If anything, it's C++'s fault for defining a function that already existed in Arduino, not Arduino's fault for defining a function that already existed in C++.)

[Koepel]

The c math with the round function (with floating point parameter and floating point return) is at least C99 as far as I know. I hope you don't mind, but I stick with my strong words: "really ruins the math library". It's a matter of principle. The math library as a whole should be reliable. In my opinion it is not okay to mess around with standardized math functions.

[PaulStoffregen]

avr-libc still does not fully implement C99, and until only a few years ago it lacked many of the C99 functions it has today. Sticking to your strong words and accusatory tone isn't helping.

[Koepel]

I'm very sorry if my tone was accusatory. I didn't mean to be like that. I hope you agree with me that a lot of effort has been put into the math library, and the round function in the math library is perfectly fine.

[cousteaulecommandant]

The math library with the round function (with floating point parameter and floating point return) is at least C99

Well yes, but Arduino is based on C++, not C.

In any case, it is true that it could be considered dropping the round() macro, specially since std::round() seems to work better, as @facchinm pointed out. (And personally I'd get rid of all these macros entirely, or at least implement them as template functions rather than macros.)
C++11 also seems to provide lround() which is the long version of round, so this functionality wouldn't be lost if Arduino moved to these standard functions.
The only problem is that the name is different, so this could potentially break backwards compatibility: if someone was already using round() in their project and relying on the old (Arduino) behavior, moving to a floating point version might break things. Then again, nowhere in the Arduino documentation does it say anything about how round() behaves.

[oqibidipo]

Well yes, but Arduino is based on C++, not C.

False. It is based on C and C++.

[PaulStoffregen]

The round() macro, as it is currently implemented by Arduino, also suffers from evaluating its arguments multiple times. For an explanation, see "More Gotchas" on this page:

http://www.cprogramming.com/tutorial/cpreprocessor.html

One possible fix might look like this:

#define round(x) ({ \
  typeof(x) _x = (x); \
  (_x>=0) ? (long)(_x+0.5) : (long)(_x-0.5); \
})

But perhaps it would be best to just use the C library round(), now that it's fully supported by the modern AVR toolchain, and as far as I know all the 32 bit toolchains.

[Koepel]

Thank you PaulStoffregen.

The origin of this issue is this: http://www.arduinoforum.nl/viewtopic.php?f=9&t=2454
Someone encountered a rounding problem and I thought it must be a joke. So my first reaction was (translated): "Ha ha, april fools day. Wait a moment... it really does go wrong. Yikes! What a nasty bug".
(I hope I don't offend someone with it).

As far as I know, in 'c' and 'c++' the round function returns a floating point number which is rounded to the nearest whole number. I really don't know any other way. I didn't know the history of this, but at this point it would be good to confirm with the standard round function.

[PaulStoffregen]

This may be stating the overly obvious, but around here arguments about novice user experience and backwards compatibility with the 12 year legacy of Arduino gain a lot more traction than promoting standards compliance, especially C99, C++11, etc.

[cousteaulecommandant]

One possible fix might look like this:

#define round(x) ({ \
  typeof(x) _x = (x); \
  (_x>=0) ? (long)(_x+0.5) : (long)(_x-0.5); \
})

I'd rather use an actual function and end this macro madness:

template<typename T> long Round(T x) {
    return (x>=0) ? (long)(x+0.5) : (long)(x-0.5);
}
#define round(x) Round(x)  // for backwards compatibility

or simply

#define round(x) lround(x)

or do nothing at all, and use the standard round() and lround() functions.

[cousteaulecommandant]

I just tested the sketch size occupation and, with my biggest disbelief, the math.h version produces way smaller binaries than the macro version

@facchinm, I don't know what's going on but I got different results.
Using the code at https://gist.github.com/cousteaulecommandant/5cffbe9fdd66beca244bf9b693f7e418#file-test_round-ino (Arduino 1.8.1, compiling for Arduino Uno) I got the following results:

Implementation	y=long	y=float
macro	2684	3002
function returning long	2684	3010
function returning float	2692	3002
math.h round	2498	3094
math.h lround	2334	3054

so the standard functions save more resources if the result is assigned to a long, but the "custom" ones are better smaller if the result is assigned to a float. [edited]

PS: Then again, if the result is going to be assigned to a float, it should not be intermediately cast to long because of the potential loss of range, so only math.h's round does the right thing.

[Pharap]

@cousteaulecommandant
"some sort of language of its own that is based on C++"

Technically speaking this is false.
Arduino uses a C++11 compiler - the language it compiles is C++.
The difference is only in the functions that are available, not in the language itself.

That said, I completely support the stance that macros should be dropped in favour of the template approach (where possible).

Macros should be avoided (where possible) because they have a great many downsides.
One of these downsides is that they make the symbol they define completely unusuable for user functions (which is the reason I ended up here in the first place). Templates would solve this issue because a user-defined function with the same name would be chosen over the template if the types of the arguments were a better match. For users who know what they are doing, such an ability is invaluable.

"Then again, if the result is going to be assigned to a float, it should not be intermediately cast to long because of the potential loss of range, so only math.h's round does the right thing."

I completely agree with this. If casting is going on, the user should be aware of it at all times.
For one thing, some users may be using their Arduino boards for mathematical or scientific purposes and such a loss of precision may be catestrophic to their results.

[NicoHood]

Since we migrate byte and boolean I think we should just use the standard c library with its round() implementation too. Same for the above linked abs() function.

[matthijskooijman]

Is this still an issue? It seems that currently, no round macro is defined at all anymore and math.h is included.

Not sure if this was really intentional, since the commits are big and the commit messages do not mention these changes at all. See e7cda3c where math.h was added and 47e23cc where round was removed.

[Koepel]

That is the ArduinoAPI.h
When I look into the current version of Arduino IDE 1.8.13, then the macro for abs() and round() are still in Arduino.h.

[matthijskooijman]

Back to the original subject, @Koepel said:

That is the ArduinoAPI.h
When I look into the current version of Arduino IDE 1.8.13, then the macro for abs() and round() are still in Arduino.h.

I suspect you are looking at the AVR core? That is not yet based on this ArduinoCore-API repository, but as soon as it is converted, it should use code from this repo and the macro is, IIUC, gone in favor of using math.h. To verify this, you can already use the megaavr and samd cores, which are already based on this repo.

My original question about whether this is intentional or not still stands, though, since this change does have small compatibility implications, so it should be at least documented somewhere.