Greatly reduced error rate of WMath map() roundtrip

[dok-net]

Half, or zero errors, depending on in/out…… ranges, for round-trip mapping at the same performance.
(Based on "improved_map" from ESP8266's Servo.cpp)

[dok-net]

Here's the sketch to prove:

uint32_t expired;
uint32_t iterations;

void setup()
{
	Serial.begin(115200);
	while (!Serial);
	Serial.println("check map");
	expired = 0;
	iterations = 0;
}

constexpr int imin = 0;
constexpr int imax = 180;
constexpr int omin = 1000;
constexpr int omax = 2000;

int i = 0;
int fails = 0;

void loop()
{
	++iterations;
	i = (i + 1) % (imax + 1);

#if defined(ESP8266) || defined(ESP32)
	cli();
	volatile auto start = ESP.getCycleCount();
#else
	// Set Timer 1 to normal mode at F_CPU.
	TCCR1A = 0;
	TCCR1B = 1;
	cli();
	volatile uint16_t start = TCNT1;
#endif

	volatile auto o = map(i, imin, imax, omin, omax);

#if defined(ESP8266) || defined(ESP32)
	sei();
	volatile auto cycles = ESP.getCycleCount() - start;
#else
	sei();
	volatile uint16_t finish = TCNT1;
	uint16_t cycles = finish - start;
#endif

	expired += cycles;
	
	if (i != map(o, omin, omax, imin, imax))
	{
		++fails;
	}

	if (iterations > 100000)
	{
		Serial.print("Cycles per map(...) = ");	Serial.print(expired / iterations);
		Serial.print(", fails = ");	Serial.print(fails);
		Serial.println();
		expired = 0;
		iterations = 0;
		fails = 0;
	}
}

[PaulStoffregen]

maybe a duplicate of this?

arduino/ArduinoCore-API#51

[dok-net]

@PaulStoffregen Well, hardly - this is a PR fixing smth tangible - rounding to the nearest integer instead of down -, the other is an issue griping about this and that evilgrin

[dok-net]

Here's a more comprehensive answer :-)
The function proposed herein is:

const long dividend = out_max - out_min;
const long divisor = in_max - in_min;
const long delta = x - in_min;
return (delta * dividend + (divisor / 2)) / divisor + out_min;

This solves for result, given x where in_min <= x <=in_max:
x = in_min -> result = out_min
x = in_max -> result = out_max
in_min < x < in_max -> result is rounded to nearest integer (not to truncated integer)

I consider comparable performance to the existing map() implementation outweighs other concerns over the proposed error catching, that also seriously modifies current behavior in corner cases.
@PaulStoffregen What do you think?

[PaulStoffregen]

One month ago, I commented on the several other map() issues. It seems nobody really cares about map().

For Teensy's core library, we're moving to a slower but (hopefully) perfectly accurate version of map() where the integer version gives the same result if done with floating point using proper round-off, and where floating point is used if the input being mapped is a float. I shared the code and a test program on the other issue and also on the Arduino forum.

As to what I think, your code looks better than what Arduino is currently using. So do most of the proposals. My hunch is your round off might end up going the wrong direction when "divisor" and "dividend" aren't the same sign. But I wrote that test program to explore those sorts of issues. Best to actually test...

Whether some small error or improper round off is acceptable for certain cases in exchange for better speed is a judgement call each core library maintainer needs to make. Or not. Just keeping the same ancient code which gives correct results at the extremes but terrible distribution between seems to be the path Arduino will always take regardless of how many better alternative are proposed. Unless there is some sign of genuine interest, I'm done here. I've shared all of my work on map().

[ArminJo]

return (delta * dividend + (divisor / 2)) / divisor + out_min;

Dit you ever test it carefully?
If delta is 1 and dividend is -40 and divisor is 6 and out_min is 0 you get -6(.1) as result instead of the expected -7 👎

So you really ask yourself why your "improved" code was not accepted?
I personnally am glad, that it was not accepted.
And if you fix your bug, the codesize for map will be a lot bigger than before, leading to new improvements to remove this functionality to save space....
I hope you get a picture of the drawbacks of such improvements 😀

[ArminJo]

Please close this PR

[dok-net]

c265ddb is a trivial rebase to current master.

[dok-net]

@PaulStoffregen In response to your remarks about signedness I added a comment to express the expectation that the function does not expect limit1, limit2 like range delimiters, but that min < max, or at the least, min <= max.
@ArminJo As is now clearly stated,

dividend is -40

is the result of an undefined use. That said, I don't see which algebraic rules allow, effectively (0 * -40 + (6 / 2)) / 6 + 0, in signed integer arithmetic, to become anything else but 0.

[dok-net]

@ArminJo

Please close this PR

Certainly not warranted, at least based on your conjecture.

[ArminJo]

@dok-net You are right, I corrected my remark.
And you still think anyone will merge this?
Why not make a gist for this, for the ones who search for rounding and can live with your input restrictions?

[dok-net]

@ArminJo So now you've created another synthetic set of internal values without giving proper values for the whole function's input. Guessing your intentions, anyway, and ignoring that min < max by definition, lets compare the implementation in master:

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

which gives us (1) * (-40) / (6) + 0 equal -6. QED?

[CLAassistant]

All committers have signed the CLA.