ArduinoCore-API icon indicating copy to clipboard operation
ArduinoCore-API copied to clipboard

map() for float data type

Open rch33 opened this issue 8 years ago • 18 comments

This is related to issue 288. The Arduino.app C++ compiler accepts float arguments (no warning) to the map() function and then truncates them to integer values. This provides erroneous output that may not be immediately noticed. I think that map() should be defined as all float -- args and return (below). Particle.io's development platform has the same problem. All C++ compilers?

My float version: float mapf(float value, float istart, float istop, float ostart, float ostop) { return ostart + (ostop - ostart) * ((value - istart) / (istop - istart)); }

See http://dicks-photon-arduino.blogspot.com/

rch33 avatar May 21 '16 20:05 rch33

Shouldn't need to rename to mapf. Overloading the map() function with all floats should suffice.

Chris--A avatar May 23 '16 02:05 Chris--A

This is just a dandy example of what's wrong with C++. I've been writing C since it was developed. Like Ken Thompson, I think C++ fills a much needed gap. The bit about "String" vs. "char" is another pain in the a**.

rch33 avatar May 23 '16 13:05 rch33

I do not quite understand the point you are making.

Overloading is a very useful feature. Also, Strings, and a single char are somewhat unrelated.

Chris--A avatar May 23 '16 20:05 Chris--A

Well, maybe it's unrelated, but it may also taken into account... https://github.com/arduino/Arduino/issues/2466

q2dg avatar May 24 '16 19:05 q2dg

Shouldn't need to rename to mapf. Overloading the map() function with all floats should suffice.

Careful here. Actually what you're interested in is the RETURN value of the function, and as far as I know C++ doesn't allow overloading by return type.

Normally you want to deal with integers, so that the map() function receives an integer and yields an integer, and only integer arithmetic is used. Sometimes you may want to get a float, using the potentially slower float arithmetic. But if someone tries to do something like x = map(n, 0.0, 1.0, 0.0, 100.0); and n is an integer... then what? (I think the compiler will complain about stuff being ambiguous.)

So options are:

  • Implement map as an overloaded function, and tell users to be very careful with the types, casting when necessary.
  • Have an integer map and a float mapf.
  • ~~Always use floats (map() is functionally equivalent to long(mapf()), but the latter probably uses more resources).~~ (eww, no.)

I might be biased by C, but in this case I think the second option makes more sense, since the intent may not be clear from the arguments, and it is potentially dangerous for inexperienced programmers.

cousteaulecommandant avatar Jul 20 '16 15:07 cousteaulecommandant

To me, C++ (and other variants) fills a much-needed gap. And I was a Bell Labs when it was developed. I always treat it as plain C where possible and would avoid it all together if I weren't programming Arduinos and Particle Photons.

For Ken Thompson's view, see Ken Thompson on C++

| | | |

|

| |
| |
Ken Thompson on C++ In an interview for Coders at Work book: It certainly has its good points. But by and large I think it’s a bad l... | |

|

|

On Wednesday, July 20, 2016 11:33 AM, cousteau <[email protected]> wrote:

Shouldn't need to rename to mapf. Overloading the map() function with all floats should suffice. Careful here. Actually what you're interested in is the RETURN value of the function, and as far as I know C++ doesn't allow overloading by return type.Normally you want to deal with integers, so that the map() function receives an integer and yields an integer, and only integer arithmetic is used. Sometimes you may want to get a float, using the potentially slower float arithmetic. But if someone tries to do something like x = map(n, 0.0, 1.0, 0.0, 100.0); and n is an integer... then what? (I think the compiler will complain about stuff being ambiguous.)So options are:

  • Implement map as an overloaded function, and tell users to be very careful with the types, casting when necessary.
  • Have an integer map and a float mapf.
  • (eww, no.)
    
    I might be biased by C, but in this case I think the second option makes more sense, since the intent may not be clear from the arguments, and it is potentially dangerous for inexperienced programmers.— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

rch33 avatar Jul 20 '16 16:07 rch33

Careful here. Actually what you're interested in is the RETURN value of the function, and as far as I know C++ doesn't allow overloading by return type.

This is true only if the return type is changed (not the parameters). However what the op was requesting was the return type and parameters be a float, which provides a unique function signature and can be overloaded (A float return has no beneficial traits when all operands are integral types (not floats)).

Normally you want to deal with integers, so that the map() function receives an integer and yields an integer, and only integer arithmetic is used. Sometimes you may want to get a float, using the potentially slower float arithmetic. But if someone tries to do something like x = map(n, 0.0, 1.0, 0.0, 100.0); and n is an integer... then what? (I think the compiler will complain about stuff being ambiguous.)

If n is an integer the float version will be selected.

There is a problem when using all integers though as map originally implements the parameters as long int. This causes ambiguity as an int can implicitly cast to both a float and long int. So certainly there is a problem here, maybe not the one you pointed out, but a problem with my code none the less. This however, can be fixed using SFINAE to invalidate the float version if no parameters are a float.

This solution would be quite verbose (usage would be simple, however the code implementing it is complex). If there is interest for it, I'd certainly be keen to write it, or at least propose a change.

Chris--A avatar Jul 21 '16 14:07 Chris--A

But if someone tries to do something like x = map(n, 0.0, 1.0, 0.0, 100.0); and n is an integer... then what?

If n is an integer the float version will be selected.

Well, I got this behavior:

int foo(int a, int b, int c, int d, int e) { return 0; }
float foo(float a, float b, float c, float d, float e) { return 0; }

void setup() { }
void loop() {
  int n = 0;
  foo(n, 1.0f, 2.0f, 3.0f, 4.0f);
}
sketch_aug17a:7: error: call of overloaded 'foo(int&, float, float, float, float)' is ambiguous

If I remove the f at the end of each number it compiles. However, it seems that it is using the int version. Proof is that this code

int foo(int a, int b, int c, int d, int e) { volatile int x = 1; return 0; }
float foo(float a, float b, float c, float d, float e) { return 0; }

void setup() { }
void loop() {
  int n = 0;
  foo(n, 1.0, 2.0, 3.0, 4.0);
}

produces an output 26 bytes larger than if I move the volatile int x = 1; to the float version. (Unfortunately I don't have an Arduino board here to make a proper test, but feel free to check this.)

In short, I think just having two versions with different names would simplify things a lot. It will save headaches to both developers and users.

cousteaulecommandant avatar Aug 17 '16 18:08 cousteaulecommandant

On August 17, 2016 at 2:17 PM cousteau [email protected] wrote:

But if someone tries to do something like x = map(n, 0.0, 1.0, 0.0, 100.0); and n is an integer... then what?

If n is an integer the float version will be selected.

Well, I got this behavior:

int foo(int a, int b, int c, int d, int e) { return 0; }
float foo(float a, float b, float c, float d, float e) { return 0; }

Shouldn't you write this as

float foo(int a, float b, float c, float d, float e) { return 0; }

Peter Olson

void setup() { } void loop() { int n = 0; foo(n, 1.0f, 2.0f, 3.0f, 4.0f); }


sketch_aug17a:7: error: call of overloaded 'foo(int&, float, float, float, float)' is ambiguous


If I remove the `f` at the end of each number it compiles.  However, **it seems that it is using the int version.**  Proof is that this code
```arduino
int foo(int a, int b, int c, int d, int e) { return 0; }
float foo(float a, float b, float c, float d, float e) { volatile int x = 1; return 0; }

void setup() { }
void loop() {
  int n = 0;
  foo(n, 1.0f, 2.0f, 3.0f, 4.0f);
}

produces an output 26 bytes larger than if I move the volatile int x = 1; to the float version. (Unfortunately I don't have an Arduino board here to make a proper test, but feel free to check this.)

In short, I think just having two versions with different names would simplify things a lot. It will save headaches to both developers and users.

You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/arduino/Arduino/issues/4975#issuecomment-240499774

peabo7 avatar Aug 18 '16 03:08 peabo7

I think what you guys are looking for is C++ templates. This would ensure return types are the same as the output types and even convert types where needed.

Here's what I've whipped up in the past for another project:

template<typename T, typename T2>
inline T map(T2 val, T2 in_min, T2 in_max, T out_min, T out_max) {
		return (val - in_min) * (out_max - out_min) / (in_max - in_min) + out_min;
}

liamkinne avatar Dec 30 '17 14:12 liamkinne

Yep, maybe templates + indicating the return type as the out_min and out_max type is the way to go. My main concern was not being able to explicitly indicate the return type, only the argument types, but it is true that it could be made so that the type of out_min and out_max is used. Also, templates would allow to state this type explicitly, like map<Type>(...) (although maybe using the correct argument type would be better).

EDIT: The issue with heterogeneous types still holds: map(1, 0.0, 2.0, 0.0, 10.0) complains; I would suggest making val its own type. Or even better; let each variable have its own type. Apaprently in C++14 this is as easy as declaring auto map(auto val, auto in_min, auto in_max, auto out_min, auto out_max). For earlier C++ versions, the only way around seems to be the template.

cousteaulecommandant avatar Dec 30 '17 18:12 cousteaulecommandant

FWIW, here's the template-based map() from Teensy, where we're using C++14 dialect and type_traits.

#ifdef __cplusplus
#include <type_traits>
// when the input number is an integer type, do all math as 32 bit signed long
template <class T, class A, class B, class C, class D>
long map(T _x, A _in_min, B _in_max, C _out_min, D _out_max, typename std::enable_if<std::is_integral<T>::value >::type* = 0)
{
        long x = _x, in_min = _in_min, in_max = _in_max, out_min = _out_min, out_max = _out_max;
        if ((in_max - in_min) > (out_max - out_min)) {
                return (x - in_min) * (out_max - out_min+1) / (in_max - in_min+1) + out_min;
        } else {
                return (x - in_min) * (out_max - out_min) / (in_max - in_min) + out_min;
        }
}
// when the input is a float or double, do all math using the input's type
template <class T, class A, class B, class C, class D>
T map(T x, A in_min, B in_max, C out_min, D out_max, typename std::enable_if<std::is_floating_point<T>::value >::type* = 0)
{
        return (x - (T)in_min) * ((T)out_max - (T)out_min) / ((T)in_max - (T)in_min) + (T)out_min;
}

PaulStoffregen avatar Dec 30 '17 21:12 PaulStoffregen

// when the input number is an integer type, do all math as 32 bit signed long

Oh yes; I forgot that. If you don't cast to long, an int->int mapping has high chances of overflowing. (Example: map(150, 0,256, 0,360) would overflow int because 150*360>32767)

Honestly I think all this (rewriting all Arduino.h's function-like macros as templated functions or std:: ones) should be thoroughly discussed, maybe on a separate issue or a thread on the mailing list.

cousteaulecommandant avatar Dec 31 '17 12:12 cousteaulecommandant

I agree, and moving things to templates and whatnot will inevitably break someones code somewhere.

I've been writing my own alternative Arduino core (@PhantomEmbedded) that addresses many of these issues, but is completely incompatible because everything is based on classes rather than functions.

If someone makes a separate thread on this topic I would be happy to help.

liamkinne avatar Dec 31 '17 17:12 liamkinne

Question: is this something people still want? I could implement it, but that's a bit wasted effort if it's not needed anymore.

MeAmAnUsername avatar Mar 29 '19 01:03 MeAmAnUsername

I'm guessing that at this point, those that were bothered by this or wanted this have rolled our own. I wouldn't be opposed to seeing something official in the future though.

marmilicious avatar Apr 01 '19 07:04 marmilicious

map should be overloaded to work with DOUBLE, NOT for float!

workaround: which mapf works already for double?

dsyleixa avatar May 18 '19 11:05 dsyleixa

The template I showed above (about 3 years ago) automatically does map() using 64 bit double if your input variable is a 64 bit double. But if you give it 32 bit float, everything is done using faster 32 bit floats. And if you give it any integer, everything is computed using 32 bit long.

No need to add mapf() to the Arduino API. Templates can let you have your cake and eat it too, from one convenient map() that automatically adapts depending on the type of the user's input variable.

PaulStoffregen avatar Oct 31 '20 10:10 PaulStoffregen