wavelength slider has a bad range #763

[samreid]

From https://github.com/phetsims/phet-io/issues/763

beersLawLab.beersLawScreen.model.light.wavelengthProperty: TProperty seems to do nothing good. Sliding it does not change the wavelength, and only serves to cause the probe to report NaN. Sliding without the light emitter turned on reveals the slider has a range of 0 to 1:

For phetsims/tasks/issues/718.

Edit: now also for phetsims/tasks/issues/724.

@zepumph can you please check on this?

[pixelzoom]

wavelengthProperty is instrumented in Light.js:

    this.wavelengthProperty = new Property( solutionProperty.get().molarAbsorptivityData.lambdaMax /*nm*/, {
      tandem: tandem.createTandem( 'wavelengthProperty' ),
      phetioValueType: TNumber( { units: 'nanometers' } )
    } );

The above description seems to imply that the proxy slider doesn't change this value. You should confirm that first, because I don't believe it.

The animated gif shows that the wavelength slider doesn't update. The wavelength slider is scenery-phet WavelengthSlider. Based on the description above, it sounds like WavelengthSlider is not updating when wavelengthProperty changes.

[phet-steele]

The animated gif shows that the wavelength slider doesn't update.

If I remember correctly (and this issue was from a long time ago, so please forgive me if this is incorrect), once I begin moving the tandem slider manually, the page becomes largely unresponsive due to an extreme amount of error reporting. So no, the slider would update under normal circumstances (like if it had the correct range) and only doesn't update here due to the page seizing up.

[pixelzoom]

@phet-steele said:

... once I begin moving the tandem slider manually, the page becomes largely unresponsive due to an extreme amount of error reporting.

Do you recall (or can your reproduce) the error reporting?

[phet-steele]

I remembered only slightly correctly. You do get a bunch of errors, but they just cause the sim/wrapper to become unresponsive, not the browser page:

The errors (though they probably wouldn't happen if the range was fixed):

SimIFrameAPI Invocation Error: Error: Assertion failed
    at window.assertions.assertFunction (assert.js:22)
    at MolarAbsorptivityData.wavelengthToMolarAbsorptivity (MolarAbsorptivityData.js?bust=1504028120448:36)
    at Absorbance.js?bust=1504028120448:42
    at Array.listener (DerivedProperty.js?bust=1504028120448:67)
    at Emitter.emit2 (Emitter.js?bust=1504028120448:200)
    at Property._notifyListeners (Property.js?bust=1504028120448:204)
    at Property._setAndNotifyListeners (Property.js?bust=1504028120448:193)
    at Property.set (Property.js?bust=1504028120448:140)
    at TPropertyImpl.implementation [as setValue] (TProperty.js?bust=1504028120448:85)
    at Object.invoke (SimIFrameAPI.js?bust=1504028120448:258) Error: Assertion failed
    at window.assertions.assertFunction (http://phettest.colorado.edu/assert/js/assert.js:22:13)
    at MolarAbsorptivityData.wavelengthToMolarAbsorptivity (http://phettest.colorado.edu/beers-law-lab/js/beerslaw/model/MolarAbsorptivityData.js?bust=1504028120448:36:15)
    at http://phettest.colorado.edu/beers-law-lab/js/beerslaw/model/Absorbance.js?bust=1504028120448:42:47
    at Array.listener (http://phettest.colorado.edu/axon/js/DerivedProperty.js?bust=1504028120448:67:57)
    at Emitter.emit2 (http://phettest.colorado.edu/axon/js/Emitter.js?bust=1504028120448:200:49)
    at Property._notifyListeners (http://phettest.colorado.edu/axon/js/Property.js?bust=1504028120448:204:29)
    at Property._setAndNotifyListeners (http://phettest.colorado.edu/axon/js/Property.js?bust=1504028120448:193:14)
    at Property.set (http://phettest.colorado.edu/axon/js/Property.js?bust=1504028120448:140:16)
    at TPropertyImpl.implementation [as setValue] (http://phettest.colorado.edu/axon/js/TProperty.js?bust=1504028120448:85:25)
    at Object.invoke (http://phettest.colorado.edu/phet-io/js/SimIFrameAPI.js?bust=1504028120448:258:29)

Uncaught Error: Assertion failed: mismatched messageIndex, possible start/end mismatch
    at window.assertions.assertFunction (assert.js:22)
    at Object.end (phetioEvents.js?bust=1504028120448:160)
    at SimIFrameAPI.js?bust=1504028120448:280

[pixelzoom]

The assertion that's failing is in MolarAbsorptivityData.wavelengthToMolarAbsorptivity:

  /*
   * Maps a visible wavelength to its corresponding molar absorptivity.
   * @param {number} wavelength
   * @returns {number}
   */
  MolarAbsorptivityData.prototype.wavelengthToMolarAbsorptivity = function( wavelength ) {
36    assert && assert( wavelength >= VisibleColor.MIN_WAVELENGTH && wavelength <= VisibleColor.MAX_WAVELENGTH );
    var index = Math.floor( wavelength - VisibleColor.MIN_WAVELENGTH );
    return this.molarAbsorptivity[ index ];
  };

So something (instance-proxies?) is providing a wavelength that is outside the visible spectrum, and is therefore invalid.

[pixelzoom]

How do I determine the range of the sliders in the instance-proxies wrapper? Specifically the range of the slider associated with beersLawLab.beersLawScreen.model.light.wavelengthProperty. And in general, why aren't the ranges shown in instance-proxies?

[phet-steele]

So something (instance-proxies?) is providing a wavelength that is outside the visible spectrum, and is therefore invalid.

Yes, that slider has a range of 0 - 1 inclusive. All tandem sliders have this range as a default until overridden with a custom range (hence this issue).

[phet-steele]

why aren't the ranges shown in instance-proxies?

Ranges ARE shown when customized, but perhaps they could be shown always.

[pixelzoom]

Ranges ARE shown when customized,

I have no idea what "when customized" means.

[pixelzoom]

All tandem sliders have this range as a default until overridden with a custom range (hence this issue)

Sounds like you knew that this was the issue? If that's the case, wish someone had said so, would have save me like 45 minutes.

[phet-steele]

I have no idea what "when customized" means.

Something like this from FAMB:

 // @public - force applied to the stack of items by the pusher
    this.appliedForceProperty = new Property( 0, {
      tandem: tandem.createTandem( 'appliedForceProperty' ),
      phetioValueType: TNumber( { units: 'newtons', range: new Range( -500, 500 ) } )
    } );

Sounds like you knew that this was the issue? If that's the case, wish someone had said so, would have save me like 45 minutes.

When this issue was logged a LONG time ago, custom ranges were not supported. Now they are, and this slider should be changed,

[phet-steele]

custom ranges were not supported

That's not true, they were!

[pixelzoom]

I changed wavelengthSlider to a NumberProperty and specified its range, like this:

    this.wavelengthProperty = new NumberProperty( solutionProperty.get().molarAbsorptivityData.lambdaMax /*nm*/, {
      range: new Range( VisibleColor.MIN_WAVELENGTH, VisibleColor.MAX_WAVELENGTH ),
      tandem: tandem.createTandem( 'wavelengthProperty' ),
      phetioValueType: TNumber( {
        units: 'nanometers'
      } )
    } );

And the problem persists. Apparently PhET-iO doesn't know about NumberProperty range, or any of the other Property options that determine valid values (e.g. validValues, isValidValue,...) So you need to duplicate the range in the options to NumberProperty and in phetioValueType. That is unacceptable, issue logged in https://github.com/phetsims/phet-io/issues/1210.

So I'm stuck using Property for now, like this:

    this.wavelengthProperty = new Property( solutionProperty.get().molarAbsorptivityData.lambdaMax /*nm*/, {
      tandem: tandem.createTandem( 'wavelengthProperty' ),
      phetioValueType: TNumber( {
        units: 'nanometers',
        range: new Range( VisibleColor.MIN_WAVELENGTH, VisibleColor.MAX_WAVELENGTH )
      } )
    } );

[pixelzoom]

Fixed in above commit. @phet-steele please confirm in instance-proxies, feel free to close.

[phet-steele]

Looks beautiful.

[pixelzoom]

This is a good example of why sim-specific phet-io issues belong in sim-specific repositories. This issue was created as phetsims/phet-io#763 in October 2016. And because it wasn't in beers-law-lab, I had no idea it existed. It was moved here 1 day ago, and now it's fixed.