map function causes huge errors when mapping to small intervals due to integer truncation

[parapilot]

the map function with this implementation
return (x - in_min) * (out_max - out_min) / (in_max - in_min) + out_min;
while mathematically correct behaves not as expected due to integer math
the implementation should work using it this way (since it compensates for accuray until final division)
return ((x - in_min) * (out_max - out_min) + (out_min * (in_max - in_min))) / (in_max - in_min);

[RobTillaart]

@parapilot
You are right, map() is limited due to integer math,
However your proposal will overflow a bit easier (depending on x) as it might add two big numbers. (another limit of integer range)

And to get proper rounding it needs to add (in_max - in_min) /2

return ((x - in_min) * (out_max - out_min) + (out_min * (in_max - in_min))  + (in_max - in_min) /2) / (in_max - in_min);

This will add 3 possible big integers so might overflow even faster.

An option is to use floats to minimize over- and underflow

return out_min + round(   (float(x - in_min) * (out_max - out_min))    /   (in_max - in_min)   );

If performance is an issue you might have a look at - https://github.com/RobTillaart/FastMap

[brewmanz]

A simple fix is to use
return (x - in_min) * (1 + out_max - out_min) / (1 + in_max - in_min) + out_min;
So, for putting into bins,
map(x, 0, 99, 0, 2);
This will return 34, 33, & 33 entries, rather than 50, 49, & 1

EDIT

I didn't realise that the mins were inclusive and the maxs exclusive; that isn't clear from the documentation

[edgar-bonet]

As it stands, map(value, fromLow, fromHigh, toLow, toHigh) does a pretty good job at mapping the interval [fromLow, fromHigh) to the interval [toLow, toHigh). Notice that these are half-open intervals: the final end points of the ranges are excluded.

For instance, the line

analogWrite(out_pin, map(analogRead(in_pin), 0, 1024, 0, 256));

achieves a perfectly uniform mapping from the range of analogRead() to the range of analogWrite(): exactly 4 input values are mapped to each output value. Any attempt at “fixing” the rounding could make it very difficult to achieve such a perfect mapping.

Half-open intervals (and, specifically, open on the right side) are not something exotic. They are already present in the API of the Arduino core:

• random(min, max) returns a pseudo-random number within [min, max)
• String::substring(from, to) returns the substring consisting of the characters with indices within [from, to).

For consistency with those uses, and in order to avoid breaking existing sketches, I think it is better to keep this existing behavior. Avoiding the overflows would still be a good thing, if not too expensive.

Instead of fixing the function, I suggest fixing its documentation. Specifically, the provided example:

val = map(val, 0, 1023, 0, 255);

which gives a less than ideal mapping.

[RobTillaart]

@edgar-bonet +1

I like the mapping of Fahrenheit to Celsius and vice versa as example

fahrenheit = map(celsius, -40, 100, -40, 212);

[brewmanz]

@RobTillaart Several comments:

1. The use of map for °F to °C is the wrong use of map. From what I understand, map treats numbers as REGIONS on the real number line rather than POINTS on the line (sorry if my terminology is wrong; I trained as an engineer, not a mathematician). Hence my mention of 'bins' in my earlier post.

2. Any attempt at rounding is doomed to failure; rounding in map(analogRead(in_pin), 0, 1024, 0, 256) will result in 257 possible outputs from an input range of 0 to 1023 - unless you adopt a similar strategy to my initial flawed suggestion. This is what I mean by saying that the numbers represent regions rather than points. "Which point is it closest to?" is the wrong question; it should be "Which region does it align with?" or possibly "Which region does align with the best?" (which is not the same as "Which point is it closest to?").

3. The map implementation does not handle your stated example well. I have seen your FastMap, but of course that uses real numbers rather than integers and so is perfect for temperature conversion. The reason for the failure is how integer division rounds fractional numbers; it round closer to zero, not down, so at the point where numbers cross below in_min there is a region that is 'double-mapped' as the internal working field switches sign i.e temperatures that map to the range -39.1°C to to -40.9°C will all be treated as -40°C. Then, all fractional °C below -40 will be one degree too high.

I have a test to demonstrate this last point. You will note that all temperatures from down 212°F to 41°F map correctly, which is where the 'highest in_min' occurs; this demonstrates that my alternative mappings of °F to °C are correct. Lower than that, and only 'exact' mapping work. (BTW I have created my own FastMapInt that fixes this issue; are you interested?):

test(T0400map_FToC_OptionsComparison) {
  int f, c1, c2, c3;
  bool failed = false;
  for(f = 212; f >= 0; --f)
  {
    c1 = map(f, -40, 212, -40, 100);
    c2 = map(f, 32, 212, 0, 100);
    c3 = map(f, 41, 212, 5, 100);
    if(c1 != c2 || c1 != c3){
      failed = true;
      Serial.print(F("Mismatch at ")); Serial.print(f, DEC); Serial.print(F("°F: C1=")); Serial.print(c1, DEC); Serial.print(F(", C2=")); Serial.print(c2, DEC); Serial.print(F(", C3=")); Serial.println(c3, DEC); 
    }
  }
  assertFalse(failed);
}

which gives output of
Mismatch at 40°F: C1=4, C2=4, C3=5
Mismatch at 39°F: C1=3, C2=3, C3=4
Mismatch at 38°F: C1=3, C2=3, C3=4
Mismatch at 37°F: C1=2, C2=2, C3=3
Mismatch at 36°F: C1=2, C2=2, C3=3
Mismatch at 35°F: C1=1, C2=1, C3=2
Mismatch at 34°F: C1=1, C2=1, C3=2
Mismatch at 33°F: C1=0, C2=0, C3=1
Mismatch at 31°F: C1=-1, C2=0, C3=0
Mismatch at 30°F: C1=-2, C2=-1, C3=-1
Mismatch at 29°F: C1=-2, C2=-1, C3=-1
Mismatch at 28°F: C1=-3, C2=-2, C3=-2
Mismatch at 27°F: C1=-3, C2=-2, C3=-2
Mismatch at 26°F: C1=-4, C2=-3, C3=-3
Mismatch at 25°F: C1=-4, C2=-3, C3=-3
Mismatch at 24°F: C1=-5, C2=-4, C3=-4
Mismatch at 22°F: C1=-6, C2=-5, C3=-5
Mismatch at 21°F: C1=-7, C2=-6, C3=-6
Mismatch at 20°F: C1=-7, C2=-6, C3=-6
Mismatch at 19°F: C1=-8, C2=-7, C3=-7
Mismatch at 18°F: C1=-8, C2=-7, C3=-7
Mismatch at 17°F: C1=-9, C2=-8, C3=-8
Mismatch at 16°F: C1=-9, C2=-8, C3=-8
Mismatch at 15°F: C1=-10, C2=-9, C3=-9
Mismatch at 13°F: C1=-11, C2=-10, C3=-10
Mismatch at 12°F: C1=-12, C2=-11, C3=-11
Mismatch at 11°F: C1=-12, C2=-11, C3=-11
Mismatch at 10°F: C1=-13, C2=-12, C3=-12
Mismatch at 9°F: C1=-13, C2=-12, C3=-12
Mismatch at 8°F: C1=-14, C2=-13, C3=-13
Mismatch at 7°F: C1=-14, C2=-13, C3=-13
Mismatch at 6°F: C1=-15, C2=-14, C3=-14
Mismatch at 4°F: C1=-16, C2=-15, C3=-15
Mismatch at 3°F: C1=-17, C2=-16, C3=-16
Mismatch at 2°F: C1=-17, C2=-16, C3=-16
Mismatch at 1°F: C1=-18, C2=-17, C3=-17
Mismatch at 0°F: C1=-18, C2=-17, C3=-17
Assertion failed: (failed=true) is false, file TestIntegerMapping.ino, line 92.
Test T0400map_FToC_OptionsComparison failed.

(my apologies for terrible formatting; I can't seem to get code format working as I want)

[RobTillaart]

I know of the map problems for years, and seen this discussion more than once.
Map is part of the Arduino eco system that has the original goal to introduce people from 8-88 to the world of mechatronics (my words).

Map works well enough for this purpose, however for serious code one must ask if the errors and impact due to truncating are acceptable or not. E.g a humidity sensor in a weather station with 5% accuracy will be less problematic than a lidar used to prevent collisions.

Furthermore a question is if the mapping needed is linear e.g NTC resistor but that is a separate discussion.

1. I like the CF conversion as it is well known conversion that people will understand. and people with seroius tech curiosity will be triggered by its flaws just like it did you.

Map is a staircase function that lies under a line with a certain slope. (Pos domain story) Worst case it calculates zero points correctly, due to the systematic error. You showed very well in your test.

2. Rounding
   The map function can be improved by adding proper integer rounding. One way to do this is

P/q ==> (p+q/2)/q.

Will create its own set of problems but the average error improves quite a bit.

And yes negative numbers need to be treated with additional care.

3. Nice test,
   Wrt you better print the abs error compared to float formula. Now the code assumes no failure if the values are the same.

4. Code highlighting
   You can show code syntax highlighted by using

` ``cpp

Code

` ``

But without the space between the backquotes

5. Yes i am interested in your fast map integer version.
   If you want me to include it with the fastmap library please open an issue there. Or it can be a separate library too. Just let me know.

[RobTillaart]

Wrote a small piece of code to show the staircasing of the map function with CtoF, visualized with the IDE plotter
OK I cheated by using floats but goal was to visualize how the error looks.

void setup()
{
  Serial.begin(115200);
  for (float celsius = -20; celsius < 20; celsius += 0.2)
  {
    Serial.print(celsius * 9.0 / 5.0 + 32.0);
    Serial.print("\t");
    Serial.print(map(celsius, -40, 100, -40, 212));
    Serial.println();
  }
}

void loop(){}

Visualization of the absolute error shows that map (for this conversion) is

• systematically too low in the positive domain and (max +2.6)
• systematically too high in the negative domain. (min -1.8)

part of the problem here is that the output range is larger than the input range
in the integer domain will cause that a display jumps by 1 or by 2.

float ma = 0, mi = 0;

void setup()
{
  Serial.begin(115200);
  for (float celsius = -20; celsius < 40; celsius += 0.2)
  {
    float f = celsius * 9.0 / 5.0 + 32.0 - map(celsius, -40, 100, -40, 212);
    if (f < mi) mi = f;
    if (f > ma) ma = f;
    Serial.print(f);
    Serial.println();
  }
  Serial.println(mi);
  Serial.println(ma);

void loop(){}

[parapilot]

Thank you very much for all your input. After quite a busy time I found some time to look at my original problem. Back when I "solved" it I was quite sure it's a minor change and didn't see all the impact myself. Therefore I didn't mention the context.

Looking at the stuff with all this information it took me quite a while to remember how I came to my "fix". First of all I think RobTillaart is right that in general my suggested change is worse regarding integer overflows. (To me this was not really a problem cause I was thinking that the reason for map to use long is to prevent such cases of overflow thinking in the sense that the input values would normally be only 8 or 16 bit values and using long is some kind of explicit upcasting.)

Nevertheless I think my problem is explained / addresses by edgar-bonet's comment about the intervals. I tried to use the map function to map the analog reading of a primitive analog joystick. Doing an initial calibration to compesate the zero point offset I tried something like
uint16_t value = analogRead(joystick_pin);
int8 position = (value < cal) ? map(value, 0, cal, -100, 0) : map(value, cal, 4095, 0, 100);
due to the way the integer truncation applies this leads to position being -1 starting from value = cal - 1 but 1 only for value > cal + "bin_size" thus making the zero position asymmetric and thus very sensitive for noise on the analog signal.

With my change this behavior is changed because the late division shifts truncation to the end of the calculation. Thus I get symmetric behavior around the zero point but to the cost that the uniformity is lost since I get exactly one time the border value compared to bin_size each value inbetween. And with my double mapping around zero even the double amount of zeros. Which is wanted for my usecase but indeed absolutely no general math function.

With this said I also think the current implementation of the map function is as correct / general as possible for integer math. But I agree that it could help to explain the behavior better in the documentation. I don't have a really straight forward idea, but I think the case brewmanz mentioned map(x, 0, 99, 0, 2); could help to show the importance of treating the intervals as open and check / limit the input value accordingly.

[PaulStoffregen]

Maybe this issue should be merged with these 3 others?

arduino/ArduinoCore-API#46

arduino/ArduinoCore-API#51

arduino/ArduinoCore-API#71

[per1234]

Thanks @PaulStoffregen. I will close this as a duplicate of arduino/ArduinoCore-API#51

Thanks to everyone for the excellent discussion here I will add a reference from the other issue to make sure it is not missed by interested parties. If you have additional information to share, please feel free to comment on arduino/ArduinoCore-API#51.

I don't have permissions in the arduino/ArduinoCore-API repo, so I don't do any maintenance on that issue tracker. So if any of the other issues Paul referenced need to be closed as duplicates then someone else will need to handle that.

[PaulStoffregen]

It's unfortunate issues can't be actually merged somehow.

[PaulStoffregen]

Regarding the visualizations above, the conversion from float to integer uses truncation rather than proper round off, so the plots show errors which are the sum of map()'s errors and the round-off error of passing a float to an integer input.

I would recommend testing with this:

void setup() {
  Serial.begin(115200);
  while (!Serial) ; // wait
  for (float celsius = -20; celsius < 20; celsius += 0.2) {
    Serial.print(celsius * 9.0 / 5.0 + 32.0);
    Serial.print("\t");
    int celsius_int = roundf(celsius);
    Serial.print(map(celsius_int, -40, 100, -40, 212));
    Serial.println();
  }
}

void loop() {}

[per1234]

It's unfortunate issues can be actually merged somehow.

I also wish for a true issue merge capability like the one we use all the time on the Arduino forum. As far as I know, the only way to merge on GitHub would be making a reply in the target thread with a copy of each comment in the source thread.

We have done something similar to that in cases where we needed to transfer an open issue or PR from one repository to another where there wasn't access to GitHub's issue transfer feature (either prior to the feature's introduction or because the transfer exceeds the limitations imposed on that feature).

I considered doing that manual copy for the comments here, but wasn't sure whether it was appropriate or not. If you think it would be best, I'm happy to do it.

[PaulStoffregen]

Likewise for the error plotting example above, the large jump in error is due to the program's use of truncation rather than proper round off before map() gets the input.

Here's a version which converts from float to int using proper rounding before map().

float ma = 0, mi = 0;

void setup() {
  Serial.begin(115200);
  for (float celsius = -20; celsius < 40; celsius += 0.2) {
    int celsius_int = roundf(celsius);
    float f = celsius * 9.0 / 5.0 + 32.0 - map(celsius_int, -40, 100, -40, 212);
    if (f < mi) mi = f;
    if (f > ma) ma = f;
    Serial.print(f);
    Serial.println();
  }
  Serial.println(mi);
  Serial.println(ma);
}

void loop(){}

With this version, the huge jump in error disappears. That wasn't map(). It's the result of improper round-off before map().

Of course, map() still has problems. Anyone can easily see the difference is mostly above zero in this plot. Ideally the difference from ideal due to integer precision should be centered around zero.

[PaulStoffregen]

FWIW, here is the result using the map() function from Teensy's core library, which is essentially the same code I posted on those other map() issues and on the Arduino forum.

[RobTillaart]

@PaulStoffregen
looks better!

[PaulStoffregen]

I kinda feel like we need an Anakin & Padme meme "So Arduino will fix map()"