ChartTransform modelToView doesn't handle small values

[pixelzoom]

Discovered in https://github.com/phetsims/fourier-making-waves/issues/115.

ChartTransform is not handling small numbers that approach Number.MIN_VALUE, which is 5e-324. It returns Infinity, which causes all kinds of other downstream problems.

Example:

> const transform = new phet.bamboo.ChartTransform( { viewWidth: 100 } )

> transform.setModelXRange( new phet.dot.Range( -3e-300, 3e-300 ) )
> transform.modelToView( phet.phetCore.Orientation.HORIZONTAL, 0 )
50 // correct

> transform.setModelXRange( new phet.dot.Range( -3e-310, 3e-310 ) )
> transform.modelToView( phet.phetCore.Orientation.HORIZONTAL, 0 )
Infinity // should be 0

The problem appears to in Utils.linear or Transform1.evaluate, called from modelToView:

    // For vertical, +y is usually up
    return axisOrientation === Orientation.HORIZONTAL ?
           Util.linear( transform.evaluate( modelRange.min ), transform.evaluate( modelRange.max ), 0, viewDimension, transformedValue ) :
           Util.linear( transform.evaluate( modelRange.max ), transform.evaluate( modelRange.min ), 0, viewDimension, transformedValue );

We'll probably need to some code that says "if it's this small, treat it as zero".

It would also be good to add an assertion before returning, to verify that the returned value is indeed {number} and not Infinity.

[samreid]

In JavaScript, 1/Number.MIN_VALUE evaluates to Infinity.

"if it's this small, treat it as zero".

1/0 is also Infinity.

to verify that the returned value is indeed {number}

typeof (1/0) evaluates to 'number'.

How do you recommend to proceed?

[pixelzoom]

In JavaScript, 1/Number.MIN_VALUE evaluates to Infinity. 1/0 is also Infinity.

Why is this relevant here?

How do you recommend to proceed?

This needs to be well-behaved when using small values for modelXRange, modelYRange, and the value arg to modelToView. If if it should return Infinity in cases where it makes sense, that's fine. But it's currently returning Infinity in cases that makes no sense.

[samreid]

For the example described in https://github.com/phetsims/bamboo/issues/40#issue-959309925, the linear function is:

  linear( a1, a2, b1, b2, a3 ) {
    assert && assert( typeof a3 === 'number', 'linear requires a number to evaluate' );
    return ( b2 - b1 ) / ( a2 - a1 ) * ( a3 - a1 ) + b1;
  },

with

a1=-3e-310
a2=3e-310
b1=0
b2=100
a3=0

Therefore, to transform from model to view, we divide the delta b (100) by delta a (6E-310). This works out to be

100/6E-310 => Infinity

@jonathanolson and I applied an arbitrary-precision floating point computation library for a problem in CCK, but it has a lot of baggage and should probably only be used where necessary. Would it be possible to encourage developers to use a model range larger than 1E-300?

[pixelzoom]

I guess that seems reasonable -- I ran into this in Fourier because my model was "off" and I shouldn't have had tiny values in the first place. But if it's a constraint, then the arg(s) should be verified with an assertion.

[samreid]

How about assertions on the computed values, like so?

```diff Index: js/ChartTransform.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/ChartTransform.js b/js/ChartTransform.js --- a/js/ChartTransform.js (revision 866934c419caf1de8aa0bb5dd2f8e25a771742e2) +++ b/js/ChartTransform.js (date 1630631958508) @@ -104,9 +104,13 @@ } // For vertical, +y is usually up - return axisOrientation === Orientation.HORIZONTAL ? - Util.linear( transform.evaluate( modelRange.min ), transform.evaluate( modelRange.max ), 0, viewDimension, transformedValue ) : - Util.linear( transform.evaluate( modelRange.max ), transform.evaluate( modelRange.min ), 0, viewDimension, transformedValue ); + const view = axisOrientation === Orientation.HORIZONTAL ? + Util.linear( transform.evaluate( modelRange.min ), transform.evaluate( modelRange.max ), 0, viewDimension, transformedValue ) : + Util.linear( transform.evaluate( modelRange.max ), transform.evaluate( modelRange.min ), 0, viewDimension, transformedValue ); + assert && assert( Number.isFinite( view ), 'view value should be finite' ); + assert && assert( !isNaN( view ), 'view value should be a number' ); + + return view; } /** ```

[pixelzoom]

Adding those assertions to modelToView looks like a good solution. I'm guessing there could be a similar problem with viewToModel, so perhaps similar assertions there, in case the client calls viewToModel before modelToView?

[samreid]

I added the assertions in the patch and more for the viewToModel. I wasn't sure of good variable names or assertion messages, please feel free to improve those if you have an idea about that. Ready for review.

[pixelzoom]

Looks good. I renamed a couple of local variables in the commit above.

Question about this bit of code in modelToView, which is not new:

101 let transformedValue = transform.evaluate( value );
    if ( isNaN( transformedValue ) || !Number.isFinite( transformedValue ) ) {
      transformedValue = value;
    }

Do you recall why this is needed? Should it be documented?

[samreid]

This was introduced for https://github.com/phetsims/bamboo/issues/3, and is exercised for the bamboo demo "Multiple Plots" when you select "log" scale. It seems the problem is when trying to evaluate a value outside of a function's domain. I'm not sure what to do about this case. What do you recommend?

[pixelzoom]

This was introduced for #3, and is exercised for the bamboo demo "Multiple Plots" ...

It looks like there's a bug in DemoMultiplePlots. Wasn't yScale renamed yTransform?

    const chartTransform = new ChartTransform( {
      viewWidth: 600,
      viewHeight: 400,
      modelXRange: new Range( 2, 10 ),
      modelYRange: new Range( 1, 22000 ),
      yScale: new Transform1( Math.log, Math.exp, {
        domain: new Range( 0, Number.POSITIVE_INFINITY ),
        range: new Range( 0, Number.POSITIVE_INFINITY )
      } )
    } );

I'm not sure what to do about the code that I pointed out in https://github.com/phetsims/bamboo/issues/40#issuecomment-931454309. But if this does happen, isn't this silently hiding a problem? And is the correct response really to punt on the transform and return value?

[samreid]

Thanks, I moved that part to a new issue. Can this one be closed?

[pixelzoom]

Yep, closing.