Normalize tolerance based on the value of the numerator and denominator

[zepumph]

From design discussion over in https://github.com/phetsims/ratio-and-proportion/issues/83, we don't like that for some custom ratios, you get too much range of feedback. For example with a ratio of 1/10, moving the right hand yields feedback for 100% of the range. Designers would most like feedback to range 40% of the screen. Perhaps normalizing on the numerator and denominator would make the tolerance not change so drastically based on the ratio.

[zepumph]

I was able to shape the tolerance by normalizing on the size of the target ratio. This worked very well for the denominator, solving the 1/10 case as described above. The issue that we keep coming back to is that this reciprocally effects the left hand also, so when we shrink the range of fitness for the right value, it does the same on the left value. Again we are constrained by everything discussed in #14, where because we support bimodal interaction, we have to base the fitness calculation on the ratio, and not the "other value".

Here is a patch working for the second screen that demonstrated how shrinking the right value's fitness also shrinks the left value:

```diff Index: js/common/model/RatioAndProportionModel.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- js/common/model/RatioAndProportionModel.js (revision 3ba3f2370b399af053549b3b1acaac9999832625) +++ js/common/model/RatioAndProportionModel.js (date 1595270202483) @@ -14,11 +14,11 @@ import Utils from '../../../../dot/js/Utils.js'; import Vector2 from '../../../../dot/js/Vector2.js'; import Vector2Property from '../../../../dot/js/Vector2Property.js'; -import RatioAndProportionQueryParameters from '../../common/RatioAndProportionQueryParameters.js'; +import Fraction from '../../../../phetcommon/js/model/Fraction.js'; import ratioAndProportion from '../../ratioAndProportion.js'; import RatioAndProportionConstants from '../RatioAndProportionConstants.js'; -// The threshold for velocity of a moving ratio value to indivate that it is "moving." +// The threshold for velocity of a moving ratio value to indicate that it is "moving." const VELOCITY_THRESHOLD = .01; // The value in which when either the left or right value is less than this, the ratio cannot be "in proportion" @@ -32,8 +32,8 @@ constructor( tandem ) { // The desired ratio of the left value as compared to the right value. As in 1:2 (initial value). - this.ratioProperty = new NumberProperty( .5 ); - this.toleranceProperty = new NumberProperty( RatioAndProportionQueryParameters.tolerance ); + this.ratioProperty = new Property( new Fraction( 2, 4 ) ); + this.toleranceProperty = new NumberProperty( .2 ); // @public this.valueRange = new Range( .0001, 1 ); // Do avoid devide-by zero errors @@ -83,15 +83,17 @@ if ( isNaN( currentRatio ) ) { return 0; } - const desiredLeft = ratio * rightValue; + + const desiredLeft = ratio.value * rightValue; const fitnessError = Math.abs( desiredLeft - leftValue ); - const fitness = this.fitnessRange.max - Utils.clamp( fitnessError / tolerance, this.fitnessRange.min, this.fitnessRange.max ); + const fitness = this.fitnessRange.max - Utils.clamp( ( fitnessError * ratio.denominator ) / tolerance / this.valueRange.getLength(), this.fitnessRange.min, this.fitnessRange.max ); // If either value is small enough, then we don't allow an "in proportion" fitness level, so make it just below that threshold. if ( fitness >= this.fitnessRange.max - RatioAndProportionConstants.IN_PROPORTION_FITNESS_THRESHOLD && ( leftValue <= NO_SUCCUSS_VALUE_THRESHOLD || rightValue <= NO_SUCCUSS_VALUE_THRESHOLD ) ) { return this.fitnessRange.max - RatioAndProportionConstants.IN_PROPORTION_FITNESS_THRESHOLD - .01; } + console.log( fitness ); return fitness; }, { isValidValue: value => this.fitnessRange.contains( value ) Index: js/explore/view/ExploreScreenSummaryNode.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- js/explore/view/ExploreScreenSummaryNode.js (revision 3ba3f2370b399af053549b3b1acaac9999832625) +++ js/explore/view/ExploreScreenSummaryNode.js (date 1595269782322) @@ -17,7 +17,7 @@ * @param {Property.} ratioFitnessProperty * @param {Property.} leftValueProperty * @param {Property.} rightValueProperty - * @param {Property.} ratioProperty + * @param {Property.} ratioProperty * @param {Property.} gridViewProperty * @param {RatioDescriber} ratioDescriber * @param {Map.} ratioToChallengeNameMap - map from target ratio to name of challenge @@ -53,7 +53,7 @@ Property.multilink( [ ratioProperty, gridViewProperty, ratioFitnessProperty, leftValueProperty, rightValueProperty ], currentTargetRatio => { stateOfSimNode.innerContent = StringUtils.fillIn( ratioAndProportionStrings.a11y.explore.screenSummary.qualitativeStateOfSim, { ratioFitness: this.ratioDescriber.getRatioFitness( false ), // lowercase - currentChallenge: ratioToChallengeNameMap.get( currentTargetRatio ) + currentChallenge: ratioToChallengeNameMap.get( currentTargetRatio.value ) } ); leftHandBullet.innerContent = StringUtils.fillIn( ratioAndProportionStrings.a11y.leftHandBullet, { position: ratioDescriber.getLeftQualitativePointerPosition() Index: js/explore/view/ExploreScreenView.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- js/explore/view/ExploreScreenView.js (revision 3ba3f2370b399af053549b3b1acaac9999832625) +++ js/explore/view/ExploreScreenView.js (date 1595270374761) @@ -4,6 +4,7 @@ * @author Michael Kauzmann (PhET Interactive Simulations) */ +import Fraction from '../../../../phetcommon/js/model/Fraction.js'; import Node from '../../../../scenery/js/nodes/Node.js'; import ComboBox from '../../../../sun/js/ComboBox.js'; import SoundClip from '../../../../tambo/js/sound-generators/SoundClip.js'; @@ -51,16 +52,18 @@ innerContent: ratioAndProportionStrings.a11y.explore.ratioChallenges, tagName: 'h3' } ); + + // TODO: it isn't totally clear how best to map from Fraction to number here, to support ratioToChallengeNameMap const comboBox = new ComboBox( [ - new ChallengeComboBoxItem( ratioToChallengeNameMap.get( 1 / 2 ), 'green', 1 / 2, { + new ChallengeComboBoxItem( ratioToChallengeNameMap.get( 1 / 3 ), 'green', new Fraction( 2, 4 ), { soundPlayer: soundGenerators[ 0 ], a11yLabel: ratioAndProportionStrings.challenge1 } ), - new ChallengeComboBoxItem( ratioToChallengeNameMap.get( 1 / 3 ), 'blue', 1 / 3, { + new ChallengeComboBoxItem( ratioToChallengeNameMap.get( 1 / 3 ), 'blue', new Fraction( 1, 3 ), { soundPlayer: soundGenerators[ 1 ], a11yLabel: ratioAndProportionStrings.challenge2 } ), - new ChallengeComboBoxItem( ratioToChallengeNameMap.get( 3 / 4 ), 'magenta', 3 / 4, { + new ChallengeComboBoxItem( ratioToChallengeNameMap.get( 3 / 4 ), 'magenta', new Fraction( 3, 4 ), { soundPlayer: soundGenerators[ 2 ], a11yLabel: ratioAndProportionStrings.challenge3 } ) Index: js/create/view/CreateScreenView.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- js/create/view/CreateScreenView.js (revision 3ba3f2370b399af053549b3b1acaac9999832625) +++ js/create/view/CreateScreenView.js (date 1595268134364) @@ -73,7 +73,7 @@ } ); Property.multilink( [ numeratorProperty, denominatorProperty ], ( leftValue, rightValue ) => { - model.ratioProperty.value = leftValue / rightValue; + model.ratioProperty.value = new Fraction( leftValue, rightValue ); } ); const myChallengeContent = new HBox( { ```

[zepumph]

After discussing this with @samreid, we were able to classify the problem into trying to solve a function based on each value and the target ratio: f( x1, x2, t). This helped us note that there were two ways to solve this, each treating a different value as the "optimal" or "target" value, and then calculating how "off" the other value was, like this:

      const f1 = ( x1, x2opt, t ) => 1 - 0.5 * Math.abs( x1 - t * x2opt );
      const f2 = ( x1opt, x2, t ) => 1 - 0.5 * Math.abs( x2 - x1opt / t );

We then used this to note the paradox in how each state can yield two different values. There wasn't an obviously "best way" to solve this, but we experimented with always using the max fitness (like giving the benefit of the doubt for the algorithm). This yielded the same issues as to why this issue was created. In which smaller target ratios, like 1/10, yielded a much larger fitness range.

Then we looked at using the min fitness from each of them. This got us as far as the above patch did, in which the right value always has 40% of the range giving fitness feedback, but the left value could be quite a bit smaller than that. The aforementioned cases are only for ratios smaller than 1 (otherwise the hands' behavior is reversed).

Still this was nice enough that I decided to commit using the min fitness between the two.

I think this could be improved by potentially experimenting with using this as the default behavior, and then having a single interaction drag give priority to one of the fitness functions. That would likely yield some cases where the fitness unexpectedly jumped a large amount, but I think it is worth exploring.

Thanks @samreid for all your help today!

[samreid]

Ideas from using deltas:

```diff Index: js/common/model/RatioAndProportionModel.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- js/common/model/RatioAndProportionModel.js (revision 1fb0ae747e8d824b93363279d7eb56434275fdba) +++ js/common/model/RatioAndProportionModel.js (date 1595285586620) @@ -24,6 +24,8 @@ // The value in which when either the left or right value is less than this, the ratio cannot be "in proportion" const NO_SUCCUSS_VALUE_THRESHOLD = .05; +// import BigNumber from '../../../../bignumber.js/bignumber.mjs'; + class RatioAndProportionModel { /** @@ -72,42 +74,79 @@ // @public {DerivedProperty.} // How "correct" the proportion currently is. Can be between 0 and 1, if 1, the proportion of the two values is // exactly the value of the ratioProperty. If zero, it is outside the tolerance allowed for the proportion. - this.ratioFitnessProperty = new DerivedProperty( [ - this.leftValueProperty, - this.rightValueProperty, - this.ratioProperty, - this.toleranceProperty - ], ( leftValue, rightValue, ratio, tolerance ) => { - assert && assert( rightValue !== 0, 'cannot divide by zero' ); - assert && assert( !isNaN( leftValue / rightValue ), 'ratio should be defined' ); + // this.ratioFitnessProperty = new DerivedProperty( [ + // this.leftValueProperty, + // this.rightValueProperty, + // this.ratioProperty, + // this.toleranceProperty + // ], ( leftValue, rightValue, ratio, tolerance ) => { + // assert && assert( rightValue !== 0, 'cannot divide by zero' ); + // assert && assert( !isNaN( leftValue / rightValue ), 'ratio should be defined' ); + // + // // constant to help achieve feedback in 40% of the visual screen height. + // const toleranceFactor = 0.5; + // + // // fitness according to treating the right value as "correct" in relation to the target ratio + // const fLeft = ( left, rightOptimal, targetRatio ) => 1 - toleranceFactor * Math.abs( left - targetRatio * rightOptimal ); + // + // // fitness according to treating the left value as "correct" in relation to the target ratio + // const fRight = ( leftOptimal, right, targetRatio ) => 1 - toleranceFactor * Math.abs( right - leftOptimal / targetRatio ); + // + // // Calculate both possible fitness values, and take the minimum. In experience this works well at creating a + // // tolerance range that is independent of the target ratio or the positions of the values. This algorithm can + // // make the tolerance of the left value a bit too small for small target ratios. + // const getFitness = ( left, right ) => Math.min( fLeft( left, right, ratio ), fRight( left, right, ratio ) ); + // + // const unclampedFitness = getFitness( leftValue * 10, rightValue * 10 ); + // + // const fitness = Utils.clamp( unclampedFitness, this.fitnessRange.min, this.fitnessRange.max ); + // // console.log( leftValue * 10, rightValue * 10, ratio, fitness ); + // + // // If either value is small enough, then we don't allow an "in proportion" fitness level, so make it just below that threshold. + // if ( this.inProportion( fitness ) && this.valuesTooSmallForSuccess() ) { + // return this.fitnessRange.max - this.getInProportionThreshold() - .01; + // } + // return fitness; + // }, { + // isValidValue: value => this.fitnessRange.contains( value ) + // } ); + let unclampedFitness = 1; + this.ratioFitnessProperty = new NumberProperty( 1, {} ); + this.leftValueProperty.lazyLink( ( newValue, oldValue ) => { + const previousRatio = oldValue / this.rightValueProperty.value; + const proposedRatio = newValue / this.rightValueProperty.value; + const targetRatio = this.ratioProperty.value; + + const proposedError = Math.abs( proposedRatio - targetRatio ); + const previousError = Math.abs( previousRatio - targetRatio ); - // constant to help achieve feedback in 40% of the visual screen height. - const toleranceFactor = 0.5; + const sign = proposedError > previousError ? -1 : 1; - // fitness according to treating the right value as "correct" in relation to the target ratio - const fLeft = ( left, rightOptimal, targetRatio ) => 1 - toleranceFactor * Math.abs( left - targetRatio * rightOptimal ); + const dx = sign * Math.abs( newValue - oldValue ); + unclampedFitness = unclampedFitness + 1 / 2 * dx * 10; - // fitness according to treating the left value as "correct" in relation to the target ratio - const fRight = ( leftOptimal, right, targetRatio ) => 1 - toleranceFactor * Math.abs( right - leftOptimal / targetRatio ); + this.ratioFitnessProperty.value = Utils.clamp( unclampedFitness, 0, 1 ); + } ); + this.rightValueProperty.lazyLink( ( newValue, oldValue ) => { + const previousRatio = this.leftValueProperty.value / oldValue; + const proposedRatio = this.leftValueProperty.value / newValue; + const targetRatio = this.ratioProperty.value; - // Calculate both possible fitness values, and take the minimum. In experience this works well at creating a - // tolerance range that is independent of the target ratio or the positions of the values. This algorithm can - // make the tolerance of the left value a bit too small for small target ratios. - const getFitness = ( left, right ) => Math.min( fLeft( left, right, ratio ), fRight( left, right, ratio ) ); + const proposedError = Math.abs( proposedRatio - targetRatio ); + const previousError = Math.abs( previousRatio - targetRatio ); - const unclampedFitness = getFitness( leftValue * 10, rightValue * 10 ); + const sign = proposedError > previousError ? -1 : 1; - const fitness = Utils.clamp( unclampedFitness, this.fitnessRange.min, this.fitnessRange.max ); - // console.log( leftValue * 10, rightValue * 10, ratio, fitness ); + const dx = sign * Math.abs( newValue - oldValue ); + unclampedFitness = unclampedFitness + 1 / 2 * dx * 10; - // If either value is small enough, then we don't allow an "in proportion" fitness level, so make it just below that threshold. - if ( this.inProportion( fitness ) && this.valuesTooSmallForSuccess() ) { - return this.fitnessRange.max - this.getInProportionThreshold() - .01; - } - return fitness; - }, { - isValidValue: value => this.fitnessRange.contains( value ) + this.ratioFitnessProperty.value = Utils.clamp( unclampedFitness, 0, 1 ); } ); + + this.ratioFitnessProperty.debug( 'fitness' ); + + // this.rightValueProperty.debug( 'right' ); + // this.leftValueProperty.debug( 'left' ); // @public - true before and until first user interaction with the simulation. Reset will apply to this Property. this.firstInteractionProperty = new BooleanProperty( true ); ```

[zepumph]

@samreid and @jonathanolson helped further with this, and we got very far with an approach that uses deltas. I ended up committing because it is generally working well, although there are a few strange pieces that still need to be worked out. I also committed tests. @samreid a couple tests are commented out, because there seems to be an edge case where fitness can get greater than 1. I don't think this should be able to happen, and we should catch these cases and try to solve it. This is all I can work on this tonight, but I'll go ahead again tomorrow.

[zepumph]

I also just noted that we are not yet supporting when the target ratio changes. We should link and recalculate fitness accordingly there. This may also fix the reset case too.

[zepumph]

After sleeping on this for a night. I'm pretty sure that solving tolerance by deltas won't work for two reasons. Both of thesehas the paradox of potentially having two different fitness values depending on what ratio value you consider the "ground truth"

1. Moving one value such that the fitness is no longer 1, then moving the other value. In this case, the dx is 1 when moving either value, but that centers around different points. Take the following as an example of the shortcomming:
   • Starting configuration of left:2, right:4, ratio:1/2
   • Move left to left:1, fitness is now at .5, because you moved one grid line.
   • Move right to right:3. This is the same distance that yields a fitness increase .5, so the algorithm thinks that we are in proportion at left:1 right:3 ratio:1/2.
   • This weirdness can be further demonstrated by trying to move to right:2. This further increases the fitness, to 1.5 (when unclamped) because it is still going towards the correct ratio. Buggy!
2. When switching the ratioProperty. During this, we lose the context of where we came from, and so we don't know what fitness algorithm (based on left or right) to use. If we change the ratioProperty, acting like one is the base of the ratio, and then next move the other value, then the dx values will be offset by a the difference between the ideal ratios of the two based on the target ratio. Example:
   • begin at setting: left:3 right:3 ratio:1/2 - fitness should be at zero
   • set the ratio to ratio:7/8, now the fitness could be two different things (paradox)
   • If based on the left value, then the right value would ideally be: 24/7
   • If based on the right value, then the left value would ideally by 21/8
   • By having two possible solutions, we have made it impossible to determine what the fitness should be, as if we pick one with any heuristic, it will break functionality if we then move the opposite value next.

With this in mind. I am going to revert "by deltas" commit, along with the tests. and instead work towards a solution that has trade-offs. Likely something based on https://github.com/phetsims/ratio-and-proportion/commit/1fb0ae747e8d824b93363279d7eb56434275fdba

[zepumph]

I reverted the delta-related algorithm above. Now the algorithm is back to the description explained in https://github.com/phetsims/ratio-and-proportion/issues/91#issuecomment-661268791. We have successfully shrunk the range of fitness to always be 40% of the screen whe moving the right value. This yields quite small ranges for the fitness when moving the left value for small ratios like 1/10. @emily-phet @BLFiedler please review on master.

[samreid]

Some notes from another possibility:

  // Going from 2/4 to 1/4 is very different than going from 2/4 to 1/3
  const fitness = ( x1, x2, targetFraction ) => {
    const actualFraction = x1 / x2;

    // equal footing for being too high or too low compared to target???
    const fitness = Math.min( actualFraction / targetFraction, targetFraction / actualFraction );

    // Sharpen so it's not so diffuse for 1:10
    return Math.pow( fitness, 3 );
  }

[samreid]

We also discussed making a 3d plot of the function (for fixed targetFraction) to better visualize the behavior over all ranges.

[emily-phet]

@zepumph I think it looks good to me. I think by the time someone is deeply exploring 1:10, they are most likely to be in a place where they are confirming their understanding, and likely will need less feedback.

@BLFiedler any additional thoughts?

It would be cool to see a 3D plot of the function though...

[brettfiedler]

@zepumph I think it looks good to me. I think by the time someone is deeply exploring 1:10, they are most likely to be in a place where they are confirming their understanding, and likely will need less feedback.

Agreed. I mostly stuck to talking about 1:10 because it was the extreme case, but I think it makes the transition between 1:2 and 1:3 on screen 1 more cohesive as well. Thanks for putting in so much effort to tackle this @zepumph and @samreid!

Nothing more on this particular take on tolerance from me.

@zepumph, It's a little unclear from Sam's last comment if there was any more to do here. Assigning to you to close if not.

[zepumph]

@samreid and I spent a bit more time plotting some of the functions for different fitness algorithms. We used Mathematica.

How to read these: the "flat" coordinates are the left/right values, and the height is the fitness, from 0 to 1. Note that all of these are plotted with the target ratio as a constant: 1/2. Please ignore the spikey ridge tops, this is just due to an incomplete sampling size.

Here is the what master looks like. This has been signed off on as design. Key notes here are how larger manifestations of the same ratio (i.e. 4/8) have the same slope on either side of the ridge as does smaller ones (i.e. 1/2):

---

Here is what https://github.com/phetsims/ratio-and-proportion/issues/91#issuecomment-662545967 looks like. First without the smoothing of taking the cube:

Notice how asymmetric it is for larger values vs higher. This is the behavior of how moving the right value when finding the target ratio of 1/10 can take up the entire value range. Next here is the same function with the "cubed smoothing" applied to it, still you have inconsistencies, but the slope on either side of the mountain ridge is steeper.

---

We finally discussed the downside of the current function (implemented on master in the first graphs). The issue is that the fitness range is not symmetric when altering each value. You can see this visually by looking at a given point on the ridge top, and counting the number of lines in an east/west direction to 0 fitness and comparing it to the north/south version. It is easiest to see in the second picture of this comment. When the target ratio is 1, the distance along axis are the same:

So for this function, it seemed dependent on the ratio, and we can't change the tolerance in a way that would alter this feedback.

From this exploration, we were not able to rule out the possibility of a perfect algorithm out there in the world, but we were not able to have any breakthroughs beyond visualizing the same work we have done earlier in this issue. With this in mind, coupled with the fact that master has already gotten the go-ahead, I think the best path forward is to call it a day, and begin to clean up what is currently in place.

Please speak your mind if you disagree! I'm happy to continue working on this, though I am under the impression that this is quickly sinking in my priority list.

Keeping open until clean up can occur.

[samreid]

One way to compare the slopes in each direction is to take derivatives in each dimension. For the function in master:

df/dleft = -toleranceFactor OR toleranceFactor/targetRatio (depending on which side takes the min) 
df/dright = toleranceFactor*targetRatio OR -toleranceFactor (depending on which side takes the min)

This shows why putting targetRatio=1 yielded equal slopes in each direction.

If the design in master had not been approved, then we might use the idea of analytical derivatives to help design a function that had equal slopes in each dimension, but it's unclear if/how we could do that and also achieve the other constraints of the fitness function.

I'm not recommending any further work by this comment, just wanted to jot down the analytical derivatives of the solution in master to help with our understanding of the function, and in case we come back to it in the future.

[zepumph]

Thanks @samreid, I agree with you. So far designers seem to like the current behavior. I'm going to close this issue, and we can reopen if there are problems found with the current approach.