Defect in long map() function?--integer scaling

There appears to be a defect, or at least a trap in the long map() function.

From what I read, version 11 introduced a long map function that was based on a float function. The problem I see is the scaling. In the float world, [0.0,7.0]-->[0.0,3.0] is a 3/7 scaling because we are mapping 7 unit intevals to 3 unit intervals. From my perspective, in the digital world, [0,7]-->[0,3] is a 4/8 scaling since it is mapping 8 discrete items to 4 discrete items. The conversion from float to int didn't seem to adjust for the difference in what is being counted.

Using map(i,0,7,0,3) I'd like to see 0,1,2,3,4,5,6,7,... map to a nice evenly partitioned 0,0,1,1,2,2,3,3,... but due to scaling (not truncation), it maps to 0,0,0,1,1,2,2,3,... To emphasize the scaling issue, 63 in the above scenario maps to 27 instead of the expected 31.

I don't know what I'm talking about, but it seems to me that map() should be corrected as follows:

long map(long x, long in_min, long in_max, long out_min, long out_max) { return (x - in_min) * (out_max - out_min + 1) / (in_max - in_min + 1) + out_min; }

Alternatively, the documentation could warn people that think like me that they really need to supply the first out-of-range integer for the max values. That is, if one wants to map [0-7] to [0,3] as discrete values then they should specify map(i,0,7*+1,0,3+1*), but that seems like a workaround.

Am I thinking right?

Afterthought: the mappings appear to be "open interval" mappings. That is [0,7)[7,14)...-->[0,3)[3,6).... Digitally, we are dealing with closed sets eg {0,1,2,3,4,5,6,7}, so if a user wants to use an open-interval-related function on that closed set, they'd have to simulated it with an open interval [0,8) to be successful. I suspect we don't want users to have to convert their closed sets to open intervals, but should provide a closed set mapping

I'm not sure why you think an input of 63 to a map function, that maps numbers in the range of 0 to 7 to a range from 0 to 3, should return anything but an error.

The documentation states that out of range arguments are allowed.

Does not constrain values to within the range, because out-of-range values are sometimes intended and useful. The constrain() function may be used either before or after this function, if limits to the ranges are desired.

Clarification: the 63 wasn't really critical to the discussion, but was used only to put the magnifying glass on the scaling issue, distinguishing it from a truncation issue.

I'm not sure why you think an input of 63 to a map function, that maps numbers in the range of 0 to 7 to a range from 0 to 3, should return anything but an error.

For now, maybe let's just say I don't expect an error because the documentation provides a good reason:

Does not constrain values to within the range, because out-of-range values are sometimes intended and useful.

Respectfully, I don't want to get side-tracked on the valid-range issue--the scaling issue remains even if restricted to in-bounds values, but perhaps not as obvious. If I'm thinking of this function's purpose correctly, incorrect scaling is why 0 is over-represented in the introductory example.

It appears to me that the function is behaving correctly.

(3 * 0)/7 = 0 (3 * 1)/7 = 0 (3 * 2)/7 = 0 (3 * 3)/7 = 1 (3 * 4)/7 = 1 (3 * 5)/7 = 2 (3 * 6)/7 = 2 (3 * 7)/7 = 3

If that's not the behavior you want, you can write your own function. The overhead of calling a function is the same, regardless of whether it's your function or someone else's.

If that's not the behavior you want, you can write your own function.

Maybe that was snyde, I can't tell. Yes, I can write my own function, and yes, the processor does multiply by 3/7 as instructed. The point is that the processor should probably be instructed to multiply by 4/8 instead of 3/7.

Rather than snyde, off-the-cuff remarks. Let's seriously consider quality and safety. If the mapping is incorrect by industry standards, then its poison, and who knows how many Mars orbiters will crash.

If the mapping is incorrect in the context of integers, and if intentionally left that way, it's a good reason not to use Arduino for anything important.

Maybe its time to see if there is a standard integer mapping.

Hi, let me give an example. Suppose we have a 1024-member set {0,1,2,...,1023} that we want to map to a binary set {0,1} for say an if-else decision branch. I'd think rational users would expect 50% of the values to map to zero and 50% to map to 1. In the current implementation, 99.9% of the set would map to 0 and less than 0.1% of the set would map to 1.

I'd vote to deprecate the function for 'safety' reasons and make a new one with a different name. What do others think?

I certainly didn't mean to sound snide. I was pointing out that there is a function provided with a reasonable explanation of how it works. You want a function that behaves differently.

You are certainly free to want a different kind of mapping. You are equally free to develop that function.

All I meant to convey was that there is no real advantage, performance-wise, between using your own function versus using a Arduino-supplied function.

Thanks for explaining. Peace.

I was pointing out that there is a function provided with a reasonable explanation of how it works. You want a function that behaves differently.

Funny, I looked at the ‘reasonable explanation of how it works’ and found the definition is vague enough to cover a lot of ground. It pins down the top and bottom values, but its sort of a free-for-all in between.

[…]Re-maps a number from one range to another. That is, a value of fromLow would get mapped to toLow, a value of fromHigh to toHigh, values in-between to values in-between, etc.

My proposal and the actual implementation both satisfy that vague definition of what happens in between, but I think 99% of users would be shocked at the results of say, the Z1024-to-Z2 case.

Maybe a new function is the answer for types like me, maybe clarifying notes on what happens ‘in between’ would help too. I can’t help but think that users are not getting what they expect.
If an Arduino is ever involved in mapping a roulette game, I’m betting on zero.

edit: ok, technically, and actually, the documentation does show exactly what happens to the values in between by revealing the content of the function. I’m still betting on ‘toLow’ for the same reasons.

Are you sure you're implementing it correctly? Can we see which code EXACTLY you're using? (full code please)

I've never had a problem with the map function.. I've used it to set negative numbers as well, works like a champ! And with your example of mapping 1024 to a 0 and 1... works fine... I have a 10k pot hooked up and once it's above ABOUT halfway, the LED will turn on.. else, it's off.

Maybe I'm missing something? I tend to have some horrible ADD. :)

I've got to say I'm with OP on this one.

Unless there are any good arguments to keep it as is, that I do not see?

``````long map(long x, long in_min, long in_max, long out_min, long out_max)
{
return (x - in_min) * (out_max - out_min + 1) / (in_max - in_min + 1) + out_min;
}
``````

That's how I've always scaled integer values. That's the way my customers have always expected integer values to be scaled. (And man have I done a lot of that in my previous life!)

Another way to look at it: The current function strips away 1/2 bit of information (truncates instead of rounds). I can't think of any reason to do that.

``````void setup( void )
{
Serial.begin( 9600 );

long i;
long m;

for ( i=0; i <=1023; ++i )
{
m = map( i, 0, 1023, 0, 1 );
Serial.print( i );
Serial.print( "   " );
Serial.println( m );
}
}

void loop( void )
{
}
``````

At what value of i does m change to a one?

This is exactly what it outputs Coding, plus it stops right at 1023.

``````501   0
502   0
503   0
504   0
505   0
506   0
507   0
508   0
509   0
510   0
511   0
512   0
513   0
514   0
515   0
516   0
517   0
518   0
519   0
520   0
521   0
522   0
523   0
524   0
525   0
526   0
527   0
528   0
529   0
530   0
531   0
532   0
533   0
534   0
535   0
536   0
537   0
538   0
539   0
540   0
541   0
542   0
543   0
544   0
545   0
546   0
547   0
548   0
549   0
550   0
551   0
552   0
553   0
554   0
555   0
556   0
557   0
558   0
559   0
560   0
561   0
562   0
563   0
564   0
565   0
566   0
567   0
568   0
569   0
570   0
571   0
572   0
573   0
574   0
575   0
576   0
577   0
578   0
579   0
580   0
581   0
582   0
583   0
584   0
585   0
586   0
587   0
588   0
589   0
590   0
591   0
592   0
593   0
594   0
595   0
596   0
597   0
598   0
599   0
600   0
601   0
602   0
603   0
604   0
605   0
606   0
607   0
608   0
609   0
610   0
611   0
612   0
613   0
614   0
615   0
616   0
617   0
618   0
619   0
620   0
621   0
622   0
623   0
624   0
625   0
626   0
627   0
628   0
629   0
630   0
631   0
632   0
633   0
634   0
635   0
636   0
637   0
638   0
639   0
640   0
641   0
642   0
643   0
644   0
645   0
646   0
647   0
648   0
649   0
650   0
651   0
652   0
653   0
654   0
655   0
656   0
657   0
658   0
659   0
660   0
661   0
662   0
663   0
664   0
665   0
666   0
667   0
668   0
669   0
670   0
671   0
672   0
673   0
674   0
675   0
676   0
677   0
678   0
679   0
680   0
681   0
682   0
683   0
684   0
685   0
686   0
687   0
688   0
689   0
690   0
691   0
692   0
693   0
694   0
695   0
696   0
697   0
698   0
699   0
700   0
701   0
702   0
703   0
704   0
705   0
706   0
707   0
708   0
709   0
710   0
711   0
712   0
713   0
714   0
715   0
716   0
717   0
718   0
719   0
720   0
721   0
722   0
723   0
724   0
725   0
726   0
727   0
728   0
729   0
730   0
731   0
732   0
733   0
734   0
735   0
736   0
737   0
738   0
739   0
740   0
741   0
742   0
743   0
744   0
745   0
746   0
747   0
748   0
749   0
750   0
751   0
752   0
753   0
754   0
755   0
756   0
757   0
758   0
759   0
760   0
761   0
762   0
763   0
764   0
765   0
766   0
767   0
768   0
769   0
770   0
771   0
772   0
773   0
774   0
775   0
776   0
777   0
778   0
779   0
780   0
781   0
782   0
783   0
784   0
785   0
786   0
787   0
788   0
789   0
790   0
791   0
792   0
793   0
794   0
795   0
796   0
797   0
798   0
799   0
800   0
801   0
802   0
803   0
804   0
805   0
806   0
807   0
808   0
809   0
810   0
811   0
812   0
813   0
814   0
815   0
816   0
817   0
818   0
819   0
820   0
821   0
822   0
823   0
824   0
825   0
826   0
827   0
828   0
829   0
830   0
831   0
832   0
833   0
834   0
835   0
836   0
837   0
838   0
839   0
840   0
841   0
842   0
843   0
844   0
845   0
846   0
847   0
848   0
849   0
850   0
851   0
852   0
853   0
854   0
855   0
856   0
857   0
858   0
859   0
860   0
861   0
862   0
863   0
864   0
865   0
866   0
867   0
868   0
869   0
870   0
871   0
872   0
873   0
874   0
875   0
876   0
877   0
878   0
879   0
880   0
881   0
882   0
883   0
884   0
885   0
886   0
887   0
888   0
889   0
890   0
891   0
892   0
893   0
894   0
895   0
896   0
897   0
898   0
899   0
900   0
901   0
902   0
903   0
904   0
905   0
906   0
907   0
908   0
909   0
910   0
911   0
912   0
913   0
914   0
915   0
916   0
917   0
918   0
919   0
920   0
921   0
922   0
923   0
924   0
925   0
926   0
927   0
928   0
929   0
930   0
931   0
932   0
933   0
934   0
935   0
936   0
937   0
938   0
939   0
940   0
941   0
942   0
943   0
944   0
945   0
946   0
947   0
948   0
949   0
950   0
951   0
952   0
953   0
954   0
955   0
956   0
957   0
958   0
959   0
960   0
961   0
962   0
963   0
964   0
965   0
966   0
967   0
968   0
969   0
970   0
971   0
972   0
973   0
974   0
975   0
976   0
977   0
978   0
979   0
980   0
981   0
982   0
983   0
984   0
985   0
986   0
987   0
988   0
989   0
990   0
991   0
992   0
993   0
994   0
995   0
996   0
997   0
998   0
999   0
1000   0
1001   0
1002   0
1003   0
1004   0
1005   0
1006   0
1007   0
1008   0
1009   0
1010   0
1011   0
1012   0
1013   0
1014   0
1015   0
1016   0
1017   0
1018   0
1019   0
1020   0
1021   0
1022   0
1023   1
``````

It starts at 0.. but for some reason, I can't copy that much. (I tried in Serial monitor and Hyperterm, lol.)

But everythings 0 until 1023.

When I change it to m = map( i, 0, 1023, 0, 2 );

It changes halfway, at 512.. I expected that with 0,1.. but then it stays 1 until 1023, then it turns to 2. So yeah.. that's not cool, lol

Very interesting. My software skills still suck but I find these things fascinating. Is the problem just a case of a data 'edge' failure that were not tested for, or is the basic formula flawed? An output range of 0-1 is kind of an unusual application that might not have been considered or tested for?

Unrelated, but in this application does the function have to be defined as returning a long when the output range would fit in a byte (0-1)? It would seem a waste of SRAM space.

Lefty

retrolefty:

Is the problem just a case of a data 'edge' failure that were not tested for, or is the basic formula flawed?

It is not a case of lack of testing. It either works the way it was intended to work (but I doubt that) or it's a basic formula flaw.

An output range of 0-1 is kind of an unusual application that might not have been considered or tested for?

IDoSoLikeGreenEggsAndHam's logical argument and example also illustrate the problem just not as dramatically.

Unrelated, but in this application does the function have to be defined as returning a long when the output range would fit in a byte (0-1)?

For this application, the function could easily have been defined to return a byte. It would have saved three (or two) machine instructions (assuming my brain is working correctly).

CaptainObvious:

So yeah.. that's not cool, lol

I agree!

I think the whole issue is really one of semantics. The map function converts numbers in one range to another. The arguments are, effectively, the number of intervals in each range. The OP wants the input to be the number of values in each range.

The 0->1023 range defines 1024 intervals. The 0->1 range defines 1 interval. So, the mapping, with integer results, of most input values in the 0->1023 range to 0 is not surprising.

It's similar to the random function that takes 2 arguments - the min and max values. The documentation there clearly states that max will never be one of the returned values. Calling random(1,6) and expecting a uniform distribution of 1s, 2s, 3s, 4s, 5s, and 6s will result in disappointment.

I think that OP can achieve the results he desires by adding 1 to each range. The rest of us can get the results we expect by using the function as-is.

Perhaps more detail in the documentation is required.

Even better, in my opinion, would be to have map return a float. Then, the caller could truncate or round as required.

/me voted 'other' as in 'fixed'.

This is one of those functions I would expect to be `template`.

PaulS:

The 0->1023 range defines 1024 intervals. The 0->1 range defines 1 interval.

There are 1023 intervals in the first range; 1024 discrete values.

So, the mapping, with integer results, of most input values in the 0->1023 range to 0 is not surprising.

It is to me. It’s also useless.

The rest of us can get the results we expect by using the function as-is.

The rest of who? At this point, you’re the only one participating in this discussion who wants map to work the way it does.

Why do you want it to work the way it does? What is the benefit?

I don't really care. I've never actually used the function. I simply thought that there was a reasonable explanation for the way that it works, and that the way that it works is documented.

If the function is changed, the documentation needs to be changed, too. There may be many users who's code will change behavior when they next make an unrelated change.

Generally, that is not a good thing to do to users.

Fixing it if it does not work as advertised is one thing. "Fixing" it when it does work as advertised is another matter entirely.