DerivedProperty calls derivation function with stale values

[pixelzoom]

This problem was identified while working on phetsims/beers-law-lab#210.

In some scenarios, DerivedProperty will call its derivation function with stale values. This can happens when one (or more) of its dependencies is a DerivedProperty.

Consider this example:

var aProperty = new NumberProperty( 0 );
var bProperty = new DerivedProperty( [ aProperty ], 
  function( a ) { return a + 1; } );
var cProperty = new DerivedProperty( [ aProperty, bProperty ],
  function( a, b ) {
    console.log( 'a=' + a + ' aProperty.get()=' + aProperty.get() );
    return a + b;
} );

This console output demonstrates that cProperty's derivation function is first called with the old value of aProperty, then with the new value of aProperty:

> aProperty.set( 1 )
a=0 aProperty.get()=1
a=1 aProperty.get()=1
> aProperty.set( 2 )
a=1 aProperty.get()=2
a=2 aProperty.get()=2

The problem is inherent in the approach used to "speed up" DerivedProperty. From its constructor:

47 // @private Keep track of each dependency and only update the changed value, for speed
48 this.dependencyValues = dependencies.map( function( property ) {return property.get();} );

[pixelzoom]

@samreid @jonathanolson thoughts on how to address this?

[samreid]

I tried eliminating dependencyValues and instead using dependencies.map( function( property ) {return property.get();} ); in its place (2 occurrences). When I ran the example above I saw

a=0 aProperty.get()=0
a=1 aProperty.get()=1
a=1 aProperty.get()=1
a=2 aProperty.get()=2
a=2 aProperty.get()=2

The values are corrected because they are lazily grabbed from the property instead of cached. We still call the derivation multiple times even though it is no longer necessary, but I don't know if anything can be done about that.

[samreid]

@jonathanolson and @samreid think the dynamic version proposed in #146 (comment) seems promising, but we would like to discuss it with @pixelzoom before proceeding.

[pixelzoom]

The "dynamic version" proposed in #146 (comment) is actually what I had in mind. Up to you whether you want to proceed or wait until I return from vacation to discuss.

[pixelzoom]

... though I'm not clear on why you would need "2 occurrences" of dependencies.map( function( property ) {return property.get();} ); Seems like the existing occurrence would move inside the IIFE.

[samreid]

We still need a way to get the initial value, around line 50.

[pixelzoom]

Thanks for clarifying. @samreid feel free to proceed. This is blocking phetsims/beers-law-lab#210, which is blocking phetsims/beers-law-lab#209.

[samreid]

Committed above, @pixelzoom can you please double check it? If all is well, you can close this issue and proceed on the beer's law lab issues.

[pixelzoom]

I reviewed the commit, and it looks OK to me. And I don't see any issues in bayes. So closing.