How common is the NaN problem?

clankill3r

Reading that: https://github.com/processing/processing/issues/3314

How common is the problem caused by NaN on the forum? Cause I think it's so rare that it's not worth slowing processing down by checking for NaN's.

GoToLoop

IMO, NaN should be returned as 0 instead by map(). ~O)

clankill3r

Wouldn't that be horrible? If 0 is not in the output range than you can check if it is 0. But if 0 is in the output range then something could have gone wrong but there would be no way to tell.

GoToLoop

Well, I've never checked whether map() would return some nasty NaN. Not even knew it could!
All I know is that NaN is worse than 0. Since every operation w/ it results NaN too!

KevinWorkman

There is no "NaN problem". The map() function correctly returns NaN when you give it an invalid range.

Here is the body of the map() function:

 static public final float map(float value,
                                float start1, float stop1,
                                float start2, float stop2) {
    return start2 + (stop2 - start2) * ((value - start1) / (stop1 - start1));
  }

This can result in having a 0/0 calculation, which results in NaN.

Returning a zero instead doesn't make any sense, since zero and NaN are completely different concepts.

The presence of a NaN means that the programmer did something wrong that needs to be corrected. Getting rid of the NaN gets rid of that warning.

The map() function behaves correctly. The poster of the issue is confusing the idea of exceptions with NaN. At most Processing should output a little red warning, but even that is probably overkill.

asimes

If you find yourself in a situation where you are getting a NaN answer then you probably want to rethink how you are structuring you program

There should be no need to check for NaN unless you really have no idea what numbers you are working with due to randomness (even then you can control the range of randomness)

clankill3r

Ben Fry just closed it :)

Below the code for the new map for beta 3.3. I would love to see some benchmarks for code that has nothing to do with NaN to see how it affects performance.

static public final float map(float value,
                                float start1, float stop1,
                                float start2, float stop2) {
    float outgoing =
      start2 + (stop2 - start2) * ((value - start1) / (stop1 - start1));
    String badness = null;
    if (outgoing != outgoing) {
      badness = "NaN (not a number)";

    } else if (outgoing == Float.NEGATIVE_INFINITY ||
               outgoing == Float.POSITIVE_INFINITY) {
      badness = "infinity";
    }
    if (badness != null) {
      final String msg =
        String.format("map(%s, %s, %s, %s, %s) called, which returns %s",
                      nf(value), nf(start1), nf(stop1),
                      nf(start2), nf(stop2), badness);
      PGraphics.showWarning(msg);
    }
    return outgoing;
  }

https://github.com/processing/processing/blob/master/core/src/processing/core/PApplet.java#L4802

quark

The math functions exp, log, log10, asin, acos can also return NaN but I would not add boilerplate code for these methods so why should the map function :-?

This version of the map function has to perform three if statements which it didn't have to do before. This will slow the performance on modern pipelined CPUs but in the vast majority of cases this will not have an overall negative effect on the sketch.

colouredmirrorball

Does that slow down map() a lot?

if i would do something like this: float myVarThatIThinkIsNotZero = 0; float m = map(3, 0, myVarThatIThinkIsNotZero, 2, 5); ellipse(100, m, 30, 30); then i would probably ask myself for a very long time why my ellipse is not drawn.

I encountered such problems before but doing a println of the variables involved allowed me to quickly find the source of the NaNs and fix it. I agree it's annoying to encounter them and having something tell you what's wrong is definately convenient but IMO I don't think it's worth it to slow down map for this especially not since I use it for a lot of things in nested for loops etc.

quark

Does that slow down map() a lot?

I can't put an actual figure, but it is extremely unlikely that it would have a noticeable affect on the sketch.

I think the point that @asimes is entirely valid.

When calling a function like map() that can generate invalid values for some parameter values you have 2 options.

1) Check the return value to ensure it is valid.

2) Structure the program so that the values passed to the function can ONLY return a valid value.

Both approachs are acceptable and I have used both in my own code.

(1) is OK if the method has returns a single value to indicate that a problem occured. In the case of map this is not so good because it can return 3 invalid values, NaN, POSITIVE_INFINITY and NEGATIVE_INFINITY. Note that the invalid-value returned must be different from any possible valid-value.

(2) In the vast majority of cases the programmer is aware of the data values to be processed and can control what values are passed to the function. In the map function the user simply has to ensure that that start1 != stop1 to prevent NaNs. Unless you are working with extremely large numbers (i.e. ranges >= Float.MAX_VALUE) it is almost impossible to get INFINITY values returned.

GoToLoop

The math functions exp(), log(), log10(), asin(), acos() can also return NaN. But I would not add boilerplate code for these methods! So why should the map() function?

All of those functions you've mentioned are wrappers for corresponding 1s within Math class.
While map() isn't a wrapper. Although it doesn't necessarily mean we need to slow the latter down!

clankill3r

I made a small test to compare in speed. You have to manually change between map and map2 because i'm lazy...

I had to change the format line in the map2 method cause I don't have the nf method that accepts only 1 parameter but since the following code doesn't produce any NaN's it should not affect speed.

The results for me where an average of 3ms for the old map and an average of 9ms for the new map.

Making the new map 3 times slower...

int plotX1, plotY1, plotX2, plotY2;

ArrayList<PVector> vecs = new ArrayList<PVector>();


void setup() {
  size(600, 600);

  plotX1 = plotY1 = 50;
  plotX2 = width-plotX1;
  plotY2 = height-plotY1;

  for (int i = 0; i < 1000000; i++) {
    vecs.add(new PVector(random(width), random(height)));
  }

  for (int i = 0; i < 20; i++) {
    int start = millis();
    for (PVector v : vecs) {
      int x = (int) map2(v.x, 0, width, plotX1, plotX2);
      int y = (int) map2(v.y, 0, height, plotY1, plotY2);
    }
    int end = millis()-start;

    println(end);
  }


}


static public final float map2(float value, 
  float start1, float stop1, 
  float start2, float stop2) {
  float outgoing =
    start2 + (stop2 - start2) * ((value - start1) / (stop1 - start1));
  String badness = null;
  if (outgoing != outgoing) {
    badness = "NaN (not a number)";
  } else if (outgoing == Float.NEGATIVE_INFINITY ||
    outgoing == Float.POSITIVE_INFINITY) {
    badness = "infinity";
  }
  if (badness != null) {
    final String msg =
      String.format("map(%s, %s, %s, %s, %s) called, which returns %s");
    PGraphics.showWarning(msg);
  }
  return outgoing;
}

GoToLoop

I've got a much better performance by returning outgoing ASAP. Check it out: :-bd
I've also rearranged the order of outgoing's expressions.
Dunno whether it got us any tiny performance gains... Who knows? :P

static public final float map3(
final float value, 
final float start1, final float stop1, 
final float start2, final float stop2)
{
  final float outgoing =
    (value - start1) / (stop1 - start1) * (stop2 - start2) + start2;

  if  (outgoing != Float.NEGATIVE_INFINITY
    && outgoing != Float.POSITIVE_INFINITY
    && outgoing == outgoing)  return outgoing;

  //if (Float.isFinite(outgoing))  return outgoing;  // Java 8
  //if (outgoing == outgoing && !Float.isInfinite(outgoing))  return outgoing;

  final String badness = outgoing != outgoing? "NaN (not a number)" : "Infinity"; 

  final String msg =
    String.format("map(%s, %s, %s, %s, %s) called, which returns %s."
    , value, start1, stop1, start2, stop2, badness);

  PGraphics.showWarning(msg);
  return outgoing;
}

clankill3r

I made NaN checking optional:

static boolean CHECK_FOR_NAN = false;


void checkForNaN(boolean opt) {
  CHECK_FOR_NAN = opt;
}

static public final float map2(float value, 
  float start1, float stop1, 
  float start2, float stop2) {
  float outgoing =
    start2 + (stop2 - start2) * ((value - start1) / (stop1 - start1));

  if (CHECK_FOR_NAN) {

    String badness = null;
    if (outgoing != outgoing) {
      badness = "NaN (not a number)";
    } else if (outgoing == Float.NEGATIVE_INFINITY ||
      outgoing == Float.POSITIVE_INFINITY) {
      badness = "infinity";
    }
    if (badness != null) {
      final String msg =
        String.format("map(%s, %s, %s, %s, %s) called, which returns %s");
      PGraphics.showWarning(msg);
    }
  }
  return outgoing;
}

Having it off brings me back to 3ms again.

GoToLoop

@clankill3r, I don't think they'd add another field inside PApplet just for that!
Besides, CHECK_FOR_NAN isn't a constant. Thus it should be named sorta: checkNaN. ;;)

P.S.: CHECK_FOR_NAN can't be static either. B/c if we have multiple PApplet instances,
they shouldn't share the same state of that field! :-SS

clankill3r

Yeah I started with:

public PApplet(boolean CHECK_FOR_NAN) {
    this.CHECK_FOR_NAN = CHECK_FOR_NAN; 
}

But even then it was wrong cause it wouldn't be static either...

About the field. It could go under debug / release.

Anyway, I probably drop you map3 method on github later to see what Ben Fry will do with it.

quark

Personally I wouldn't add NaN/infinity checking code to produce a warning because where do you stop. If you test for every possible user input error in all the Processing functions the code base would increase dramatically and performance would eventually become noticeably worse.

I would change the reference text for the map function to state that the user must ensure that start1 != stop1 and that if they are equal the results are indeterminate.

This correctly puts the onus on the programmer to check that the range is valid.

GoToLoop

Indeed, even my performance attempt version is about 150% slower than Processing 2.2.1's map()! :-S

P.S.: If I change ArrayList<PVector> to PVector[], map() completes at about from previous 7 ms to 1 ms.
Although my map3() still stays at about 17 ms. However, this is now 17x slower than map()'s! 8-}

clankill3r

amen to that quark. For me processing is a sinking ship that is loosing it's speed anyway :)

quark

For me processing is a sinking ship that is loosing it's speed anyway

Interesting observation that I don't agree with, at least for the JAVA mode (I can't speak for the other modes because I don't use them).

In JAVA mode Processing is simply a wrapper for Java and functions like map are great for inexperienced programmers and the sort of thing experienced programmers would write (without the NaN/ infinity checks) if they needed it. So if Processing is slowing down then Java is slowing down and that I don't agree with.

Indeed, even my performance attempt version is about 150% slower than Processing 2.2.1's map()!

It will not be possible to include the validity check code and get close to the speed of the v2.2.1 version of map. Blame the if statements >:)

clankill3r

@quark

It has more to do with java then with processing. I just want to program with cuda, vulkan and jai :) But I will probably be around here the next coming 10 years cause some ships sink slow :D

GoToLoop

Another option is offer faster alternative methods for advanced users. :)>-
They can go "undocumented" just like many other useful methods like delay() & dataPath()! :P

static public final float map3Fast(
final float value, 
final float start1, final float stop1, 
final float start2, final float stop2)
{
  return (value - start1) / (stop1 - start1) * (stop2 - start2) + start2;
}

GoToLoop

CUDA & Vulkan are GPU hardware APIs. Java have wrappers for many APIs too!

KevinWorkman

I don't think these checks should have been added to the map() function, but then again, any user who is "advanced" enough to care about the efficiency can probably just write their own map() function. Absolutely no need to further clutter Processing's code base.