The scale is fluctuating when making the measurements

[DianaTavares]

The gif at the bottom shows the behavior of the scale in the simulation. The scale is not giving a direct value, it is fluctuating.

looks like this is not happening only with some values (probably with over 30 N)

[DianaTavares]

This still happening. I think that the problem is the contact force or some objects (like the brick in the intro screen) and when that objects are in the scale, the scale lecture also have that vibration in the digits. The scale with more problems is the one that is not in the pool.

[zepumph]

This seems related to #83, which was a bug in the stepping and force calculation specific to boat. I could see there being another awkward item like that, or something more specific to p2 itself. We'll take a look, thanks.

[zepumph]

From #157:

Some scales have fluctuating values. The gif shows some examples in Intro:

[AgustinVallejo]

Something really interesting I noticed: This doesn't occur neither in the Lab scale (with the height slider) nor when grabbing the scale... Could suggest to me that having dynamic scale movements could be triggering this, maybe the force of the scale against the ground or something?

Edit: If you drag the scale with a block on top, there's no flicker. If you drag that scale against the ground, the flickering intensifies.

[zepumph]

We still want to discuss this more. We want to understand world stepping enough to know why we get more noise when we reduce the fixed time step.

For this issue and mostly for #140, it may be a solution to convert InterpolatedProperty usages that are only for the view into something like an averaged Property. Here is a patch for that investigation if we get to a point where we want it.

Subject: [PATCH] Pass launchButtonEnabledProperty directly to the launchButton.enabledProperty, see https://github.com/phetsims/projectile-data-lab/issues/141
---
Index: js/common/DensityBuoyancyCommonQueryParameters.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/DensityBuoyancyCommonQueryParameters.ts b/js/common/DensityBuoyancyCommonQueryParameters.ts
--- a/js/common/DensityBuoyancyCommonQueryParameters.ts	(revision b1e21e5edd175ad063c795ef394ccca92948e1ed)
+++ b/js/common/DensityBuoyancyCommonQueryParameters.ts	(date 1716222924633)
@@ -60,7 +60,7 @@
   // Length (in seconds) of each internal p2 timestep. Ran into weird behavior when we had only 60 a second.
   p2FixedTimeStep: {
     type: 'number',
-    defaultValue: 1 / 120
+    defaultValue: 1 / 10
   },
 
   // Controls the maximum number of physics substeps in a single step. Usually not reached
Index: js/common/model/CumulitiveProperty.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/CumulitiveProperty.ts b/js/common/model/CumulitiveProperty.ts
new file mode 100644
--- /dev/null	(date 1716224592262)
+++ b/js/common/model/CumulitiveProperty.ts	(date 1716224592262)
@@ -0,0 +1,75 @@
+// Copyright 2019-2024, University of Colorado Boulder
+
+/**
+ * A Property that is based on the step-based interpolation between a current and previous value.
+ *
+ * @author Jonathan Olson <jonathan.olson@colorado.edu>
+ */
+
+import Property, { PropertyOptions } from '../../../../axon/js/Property.js';
+import Vector2 from '../../../../dot/js/Vector2.js';
+import optionize from '../../../../phet-core/js/optionize.js';
+import densityBuoyancyCommon from '../../densityBuoyancyCommon.js';
+
+// TODO: include how long the values take up?
+type Interpolate<T> = ( values: T[] ) => T;
+type SelfOptions<T> = {
+  interpolate: Interpolate<T>;
+};
+export type CumulatavePropertyOptions<T> = SelfOptions<T> & PropertyOptions<T>;
+
+export default class CumulativeProperty<T> extends Property<T> {
+
+  public cumulativeValues: T[] = [];
+
+  private readonly interpolate: Interpolate<T>;
+
+  public constructor( initialValue: T, providedOptions: CumulatavePropertyOptions<T> ) {
+
+    const options = optionize<CumulatavePropertyOptions<T>, SelfOptions<T>, PropertyOptions<T>>()( {}, providedOptions );
+
+    super( initialValue, options );
+
+    this.interpolate = options.interpolate;
+  }
+
+  /**
+   * Sets the next value to be used (will NOT change the value of this Property).
+   */
+  public setNextValue( value: T ): void {
+    this.cumulativeValues.push( value );
+  }
+
+  /**
+   * Sets the ratio to use for interpolated values (WILL change the value of this Property generally).
+   */
+  public resolveCumulativeValue( ratio: number ): void {
+    this.value = this.interpolate( this.cumulativeValues );
+    this.cumulativeValues.length = 0;
+  }
+
+  /**
+   * Resets the Property to its initial state.
+   */
+  public override reset(): void {
+    super.reset();
+
+    this.cumulativeValues.length = 0;
+  }
+
+  /**
+   * Interpolation for numbers.
+   */
+  public static interpolateNumber( values: number[] ): number {
+    return _.mean( values );
+  }
+
+  /**
+   * Interpolation for Vector2.
+   */
+  public static interpolateVector2( values: Vector2[] ): Vector2 {
+    return _.reduce( values, ( x, y ) => x.add( y ), new Vector2( 0, 0 ) );
+  }
+}
+
+densityBuoyancyCommon.register( 'CumulativeProperty', CumulativeProperty );
\ No newline at end of file
Index: js/common/model/DensityBuoyancyModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/DensityBuoyancyModel.ts b/js/common/model/DensityBuoyancyModel.ts
--- a/js/common/model/DensityBuoyancyModel.ts	(revision b1e21e5edd175ad063c795ef394ccca92948e1ed)
+++ b/js/common/model/DensityBuoyancyModel.ts	(date 1716224592273)
@@ -332,7 +332,8 @@
               }
             }
           } );
-          mass.scaleForceInterpolatedProperty.setNextValue( scaleForce );
+          scaleForce > 0 && console.log( scaleForce );
+          mass.scaleForceInterpolatedProperty.set( scaleForce );
         }
 
         const velocity = this.engine.bodyGetVelocity( mass.body );
Index: js/common/model/Scale.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/Scale.ts b/js/common/model/Scale.ts
--- a/js/common/model/Scale.ts	(revision b1e21e5edd175ad063c795ef394ccca92948e1ed)
+++ b/js/common/model/Scale.ts	(date 1716224592282)
@@ -29,6 +29,7 @@
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
 import Ray3 from '../../../../dot/js/Ray3.js';
 import { MassShape } from './MassShape.js';
+import CumulativeProperty from './CumulitiveProperty.js';
 
 // constants
 const SCALE_WIDTH = 0.15;
@@ -112,8 +113,8 @@
 
     super( engine, options );
 
-    this.scaleForceInterpolatedProperty = new InterpolatedProperty( 0, {
-      interpolate: InterpolatedProperty.interpolateNumber,
+    this.scaleForceInterpolatedProperty = new CumulativeProperty( 0, {
+      interpolate: CumulativeProperty.interpolateNumber,
       phetioValueType: NumberIO,
       tandem: options.tandem.createTandem( 'scaleForceInterpolatedProperty' ),
       units: 'N',

[samreid]

This patch averages the scale forces over all the post steps, and shows that averaging still shows high fluctuation (in the tenths place for an aluminum block in explore).

Subject: [PATCH] Rename "Masses" => "Mass Values", see https://github.com/phetsims/buoyancy/issues/159
---
Index: js/common/model/DensityBuoyancyModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/DensityBuoyancyModel.ts b/js/common/model/DensityBuoyancyModel.ts
--- a/js/common/model/DensityBuoyancyModel.ts	(revision b1e21e5edd175ad063c795ef394ccca92948e1ed)
+++ b/js/common/model/DensityBuoyancyModel.ts	(date 1716242315959)
@@ -332,6 +332,9 @@
               }
             }
           } );
+
+          window.scaleForces = window.scaleForces || [];
+          window.scaleForces.push( scaleForce );
           mass.scaleForceInterpolatedProperty.setNextValue( scaleForce );
         }
 
@@ -400,7 +403,7 @@
     } );
 
 
-    if ( options.usePoolScale ) {
+    if ( options.usePoolScale && false ) {
       // Pool scale
       this.scale2 = new Scale( this.engine, this.gravityProperty, {
         matrix: Matrix3.translation( 0.3, -Scale.SCALE_BASE_BOUNDS.minY + this.poolBounds.minY ),
Index: js/common/model/P2Engine.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/P2Engine.ts b/js/common/model/P2Engine.ts
--- a/js/common/model/P2Engine.ts	(revision b1e21e5edd175ad063c795ef394ccca92948e1ed)
+++ b/js/common/model/P2Engine.ts	(date 1716242390930)
@@ -110,8 +110,14 @@
    * Steps forward in time.
    */
   public step( dt: number ): void {
+    window.scaleForces = window.scaleForces || [];
+    window.scaleForces.length = 0;
+    // window.scaleForces.push( scaleForce );
     this.world.step( FIXED_TIME_STEP, dt, MAX_SUB_STEPS );
     this.interpolationRatio = ( this.world.accumulator % FIXED_TIME_STEP ) / FIXED_TIME_STEP;
+
+    const mean = _.mean( window.scaleForces );
+    console.log( mean );
   }
 
   /**

132.31159329414368
P2Engine.ts:120 132.28697419166565
P2Engine.ts:120 132.3013937473297
P2Engine.ts:120 132.3001742362976
P2Engine.ts:120 132.31159329414368
P2Engine.ts:120 132.28697419166565
P2Engine.ts:120 132.3013937473297

[samreid]

On further investigation, I believe @AgustinVallejo's original pull request is the right direction. Here is the experiment I ran to help me understand:

Subject: [PATCH] Rename "Masses" => "Mass Values", see https://github.com/phetsims/buoyancy/issues/159
---
Index: js/common/DensityBuoyancyCommonQueryParameters.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/DensityBuoyancyCommonQueryParameters.ts b/js/common/DensityBuoyancyCommonQueryParameters.ts
--- a/js/common/DensityBuoyancyCommonQueryParameters.ts	(revision b1e21e5edd175ad063c795ef394ccca92948e1ed)
+++ b/js/common/DensityBuoyancyCommonQueryParameters.ts	(date 1716252290829)
@@ -60,7 +60,7 @@
   // Length (in seconds) of each internal p2 timestep. Ran into weird behavior when we had only 60 a second.
   p2FixedTimeStep: {
     type: 'number',
-    defaultValue: 1 / 120
+    defaultValue: 1 / 120/10
   },
 
   // Controls the maximum number of physics substeps in a single step. Usually not reached
Index: js/common/model/DensityBuoyancyModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/DensityBuoyancyModel.ts b/js/common/model/DensityBuoyancyModel.ts
--- a/js/common/model/DensityBuoyancyModel.ts	(revision b1e21e5edd175ad063c795ef394ccca92948e1ed)
+++ b/js/common/model/DensityBuoyancyModel.ts	(date 1716252290810)
@@ -319,11 +319,12 @@
           contactForce = Vector2.ZERO;
         }
 
-        this.engine.resetContactForces( mass.body );
+        // this.engine.resetContactForces( mass.body );
         mass.contactForceInterpolatedProperty.setNextValue( contactForce );
 
         if ( mass instanceof Scale ) {
           let scaleForce = 0;
+          // console.log( 'mass.body.interpolatedPosition.y;', mass.body.interpolatedPosition[ 1 ] )
           this.masses.forEach( otherMass => {
             if ( mass !== otherMass ) {
               const verticalForce = this.engine.bodyGetContactForceBetween( mass.body, otherMass.body ).y;
@@ -332,6 +333,9 @@
               }
             }
           } );
+
+          window.scaleForces = window.scaleForces || [];
+          window.scaleForces.push( scaleForce );
           mass.scaleForceInterpolatedProperty.setNextValue( scaleForce );
         }
 
@@ -400,7 +404,7 @@
     } );
 
 
-    if ( options.usePoolScale ) {
+    if ( options.usePoolScale && false ) {
       // Pool scale
       this.scale2 = new Scale( this.engine, this.gravityProperty, {
         matrix: Matrix3.translation( 0.3, -Scale.SCALE_BASE_BOUNDS.minY + this.poolBounds.minY ),
Index: js/common/model/P2Engine.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/P2Engine.ts b/js/common/model/P2Engine.ts
--- a/js/common/model/P2Engine.ts	(revision b1e21e5edd175ad063c795ef394ccca92948e1ed)
+++ b/js/common/model/P2Engine.ts	(date 1716252290824)
@@ -99,19 +99,42 @@
 
     // Kill vertical-only friction to avoid edge cases, see https://github.com/phetsims/density/issues/65
     // and https://github.com/phetsims/density/issues/66
-    this.world.on( 'preSolve', ( preSolveEvent: p2.PreSolveEvent ) => {
-      preSolveEvent.frictionEquations.forEach( equation => {
-        equation.enabled = equation.t[ 0 ] !== 0;
-      } );
-    } );
+    // this.world.on( 'preSolve', ( preSolveEvent: p2.PreSolveEvent ) => {
+    //   preSolveEvent.frictionEquations.forEach( equation => {
+    //     equation.enabled = equation.t[ 0 ] !== 0;
+    //   } );
+    // } );
   }
 
   /**
    * Steps forward in time.
    */
   public step( dt: number ): void {
-    this.world.step( FIXED_TIME_STEP, dt, MAX_SUB_STEPS );
-    this.interpolationRatio = ( this.world.accumulator % FIXED_TIME_STEP ) / FIXED_TIME_STEP;
+    window.scaleForces = window.scaleForces || [];
+    window.scaleForces.length = 0;
+    // window.scaleForces.push( scaleForce );
+    // this.world.step( FIXED_TIME_STEP, dt, MAX_SUB_STEPS );
+    // this.interpolationRatio = ( this.world.accumulator % FIXED_TIME_STEP ) / FIXED_TIME_STEP;
+
+    // Ensure dt is a multiple of FIXED_TIME_STEP
+//     const FIXED_TIME_STEP = 1 / 120; // Example fixed time step
+//     const MAX_SUB_STEPS = 30; // Maximum substeps
+//
+// // Calculate the number of full steps we can take
+//     const steps = Math.floor( dt / FIXED_TIME_STEP );
+//     const adjustedDt = steps * FIXED_TIME_STEP;
+
+// Step the physics world
+    this.world.step( FIXED_TIME_STEP, FIXED_TIME_STEP*20, MAX_SUB_STEPS*100 );
+
+// Ensure no leftover in the accumulator
+//     this.world.accumulator = 0;
+//
+// // Calculate interpolation ratio if needed
+//     this.interpolationRatio = 0;
+
+    const mean = _.mean( window.scaleForces );
+    console.log( mean );
   }
 
   /**

Note in the patch, I have set the dt to be very low and make sure there is no remainder in the accumulator. This helps rule out sources of problems. In Explore => aluminum, we still have this oscillation in the model:

132.33227111399174
132.26592510938644
132.3312997072935
132.27052487432957
132.33227111399174
132.26592510938644
132.3312997072935
132.27052487432957
132.33227111399174
132.26592510938644
132.3312997072935
132.27052487432957
132.33227111399174
132.26592510938644
132.3312997072935
132.27052487432957
132.33227111399174
132.26592510938644

However, I observed that the gravity force on the scale is: 132.30N

Note that averaging the values above gives nearly that: 132.299904. The effect of the pull request is averaging across (not within) time steps. We may have to take for granted that the p2 engine has instabilities and to average across them.

[samreid]

This patch changes the scale to be STATIC or DYNAMIC based on whether the user is dragging it:

Subject: [PATCH] Rename "Masses" => "Mass Values", see https://github.com/phetsims/buoyancy/issues/159
---
Index: js/common/model/Scale.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/Scale.ts b/js/common/model/Scale.ts
--- a/js/common/model/Scale.ts	(revision b1e21e5edd175ad063c795ef394ccca92948e1ed)
+++ b/js/common/model/Scale.ts	(date 1716254327809)
@@ -82,8 +82,7 @@
 
   public constructor( engine: PhysicsEngine, gravityProperty: TProperty<Gravity>, providedOptions: ScaleOptions ) {
 
-    const bodyType = providedOptions.canMove === false ? 'STATIC' :
-                     'DYNAMIC';
+    const bodyType = 'STATIC';
 
     const options = optionize<ScaleOptions, SelfOptions, InstrumentedMassOptions>()( {
       body: engine.createBox( SCALE_WIDTH, SCALE_HEIGHT, bodyType ),
@@ -112,6 +111,19 @@
 
     super( engine, options );
 
+    this.userControlledProperty.link( userControlled => {
+
+        if ( userControlled ) {
+          console.log( 'userControlled', userControlled );
+          console.log( this.body.type );
+          this.body.type = userControlled ? p2.Body.DYNAMIC : p2.Body.STATIC;
+          this.body.mass = userControlled ? 10 : 0;
+          console.log( this.body.type );
+          this.body.updateMassProperties();
+        }
+      }
+    );
+
     this.scaleForceInterpolatedProperty = new InterpolatedProperty( 0, {
       interpolate: InterpolatedProperty.interpolateNumber,
       phetioValueType: NumberIO,
@@ -159,6 +171,22 @@
     this.stepX = xOffset;
     this.stepBottom = yOffset - SCALE_HEIGHT / 2;
     this.stepTop = yOffset + SCALE_HEIGHT / 2;
+
+    if ( Math.abs( this.body.position[ 1 ] - this.body.previousPosition[ 1 ] ) <= 1E-6
+
+         && !this.userControlledProperty.value && this.body.type === p2.Body.DYNAMIC ) {
+
+      console.log('switched back to static')
+
+      // console.log( 'position', this.body.position[ 1 ], this.body.previousPosition[ 1 ] );
+      //
+      // console.log( 'userControlled', this.userControlledProperty.value );
+      // console.log( this.body.type );
+      this.body.type = this.userControlledProperty.value ? p2.Body.DYNAMIC : p2.Body.STATIC;
+      this.body.mass = this.userControlledProperty.value ? 10 : 0;
+      // console.log( this.body.type );
+      this.body.updateMassProperties();
+    }
   }
 
   /**

It seems to work around the flickering problem. But also it fails the "Galileo's Leaning Tower of Pisa experiment" test where you toss a scale with a mass on it. The latest dev version correctly shows 0 for that case, whereas this patch incorrectly shows a weight on the scale. I do not know why, but if we can solve that and the heuristics for when to become static again, this may be a fair workaround.

[AgustinVallejo]

We kept discussing this. @samreid is going to continue reading on this. I pushed my potential solution in phetsims/density-buoyancy-common#131

[samreid]

Some experiments for my understanding of the interpolation

Subject: [PATCH] Adjust comments, mark fields as readonly, change spelling of ochre, see https://github.com/phetsims/density-buoyancy-common/issues/123
---
Index: density-buoyancy-common/js/common/model/Mass.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/density-buoyancy-common/js/common/model/Mass.ts b/density-buoyancy-common/js/common/model/Mass.ts
--- a/density-buoyancy-common/js/common/model/Mass.ts	(revision 5ace9a6bc13b0a1ba30e66332d8f6a55369eead2)
+++ b/density-buoyancy-common/js/common/model/Mass.ts	(date 1716382719910)
@@ -199,6 +199,7 @@
   public readonly gravityForceInterpolatedProperty: InterpolatedProperty<Vector2>;
   public readonly buoyancyForceInterpolatedProperty: InterpolatedProperty<Vector2>;
   public readonly contactForceInterpolatedProperty: InterpolatedProperty<Vector2>;
+  public readonly myPositionProperty: InterpolatedProperty<Vector2>;
 
   public readonly forceOffsetProperty: Property<Vector3>;
 
@@ -459,6 +460,11 @@
       phetioHighFrequency: true
     } );
 
+    this.myPositionProperty = new InterpolatedProperty( new Vector2( 0, 0 ), {
+      interpolate: InterpolatedProperty.interpolateVector2,
+      valueComparisonStrategy: 'equalsFunction'
+    } );
+
     this.forceOffsetProperty = new Property( Vector3.ZERO, {
       valueType: Vector3,
       valueComparisonStrategy: 'equalsFunction',
@@ -650,8 +656,12 @@
     this.transformedEmitter.emit();
 
     this.contactForceInterpolatedProperty.setRatio( interpolationRatio );
+    this.myPositionProperty.setRatio( interpolationRatio );
     this.buoyancyForceInterpolatedProperty.setRatio( interpolationRatio );
     this.gravityForceInterpolatedProperty.setRatio( interpolationRatio );
+
+    console.log( 'Mass.step' );
+    console.log( 'my position value', this.myPositionProperty.value, 'p2position.interpolatedPosition', this.body.interpolatedPosition );
   }
 
   /**
@@ -680,6 +690,7 @@
     this.gravityForceInterpolatedProperty.reset();
     this.buoyancyForceInterpolatedProperty.reset();
     this.contactForceInterpolatedProperty.reset();
+    this.myPositionProperty.reset();
 
     // NOTE: NOT resetting bodyOffsetProperty/forceOffsetProperty/massLabelOffsetProperty/massLabelOffsetOrientationProperty on
     // purpose, it will be adjusted by subtypes whenever necessary, and a reset may break things here.
@@ -704,6 +715,7 @@
     this.buoyancyForceInterpolatedProperty.dispose();
     this.contactForceInterpolatedProperty.dispose();
     this.internalVisibleProperty.dispose();
+    this.myPositionProperty.dispose();
     super.dispose();
   }
 
Index: sherpa/lib/p2-0.7.1.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sherpa/lib/p2-0.7.1.js b/sherpa/lib/p2-0.7.1.js
--- a/sherpa/lib/p2-0.7.1.js	(revision 975080846f42ebcab493955106748e30346ae6c8)
+++ b/sherpa/lib/p2-0.7.1.js	(date 1716379372532)
@@ -12876,6 +12876,7 @@
  * @see http://bulletphysics.org/mediawiki-1.5.8/index.php/Stepping_The_World
  */
 World.prototype.step = function(dt,timeSinceLastCalled,maxSubSteps){
+    console.log('### WORLD STEP BEGIN');
     maxSubSteps = maxSubSteps || 10;
     timeSinceLastCalled = timeSinceLastCalled || 0;
 
@@ -12904,6 +12905,8 @@
             vec2.lerp(b.interpolatedPosition, b.previousPosition, b.position, t);
             b.interpolatedAngle = b.previousAngle + t * (b.angle - b.previousAngle);
         }
+
+        console.log('p2lerp');
     }
 };
 
Index: density-buoyancy-common/js/common/model/DensityBuoyancyModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/density-buoyancy-common/js/common/model/DensityBuoyancyModel.ts b/density-buoyancy-common/js/common/model/DensityBuoyancyModel.ts
--- a/density-buoyancy-common/js/common/model/DensityBuoyancyModel.ts	(revision 5ace9a6bc13b0a1ba30e66332d8f6a55369eead2)
+++ b/density-buoyancy-common/js/common/model/DensityBuoyancyModel.ts	(date 1716382806663)
@@ -321,6 +321,7 @@
 
         this.engine.resetContactForces( mass.body );
         mass.contactForceInterpolatedProperty.setNextValue( contactForce );
+        mass.myPositionProperty.setNextValue( new Vector2( mass.body.position[ 0 ], mass.body.position[ 1 ] ) );
 
         if ( mass instanceof Scale ) {
           let scaleForce = 0;
Index: density-buoyancy-common/js/common/model/InterpolatedProperty.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/density-buoyancy-common/js/common/model/InterpolatedProperty.ts b/density-buoyancy-common/js/common/model/InterpolatedProperty.ts
--- a/density-buoyancy-common/js/common/model/InterpolatedProperty.ts	(revision 5ace9a6bc13b0a1ba30e66332d8f6a55369eead2)
+++ b/density-buoyancy-common/js/common/model/InterpolatedProperty.ts	(date 1716379312845)
@@ -53,12 +53,15 @@
   public setNextValue( value: T ): void {
     this.previousValue = this.currentValue;
     this.currentValue = value;
+
+    console.log( 'setNextValue', this.previousValue, this.currentValue );
   }
 
   /**
    * Sets the ratio to use for interpolated values (WILL change the value of this Property generally).
    */
   public setRatio( ratio: number ): void {
+    console.log( 'setRatio', ratio );
     this.ratio = ratio;
 
     this.value = this.interpolate( this.previousValue, this.currentValue, this.ratio );

[zepumph]

From conversations over in phetsims/density-buoyancy-common#132. InterpolatedProperty is largely working correctly, as it pertains to the tradeoffs in the p2 engine. I believe the best path forward for this issue is to determine how we want to process that output for the best view experience.

[samreid]

Brainstorming postprocessing that averages values, and an optional tuning:

Subject: [PATCH] Pass launchButtonEnabledProperty directly to the launchButton.enabledProperty, see https://github.com/phetsims/projectile-data-lab/issues/141
---
Index: js/buoyancy-basics/model/BuoyancyBasicsExploreModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/buoyancy-basics/model/BuoyancyBasicsExploreModel.ts b/js/buoyancy-basics/model/BuoyancyBasicsExploreModel.ts
--- a/js/buoyancy-basics/model/BuoyancyBasicsExploreModel.ts	(revision 3978f23c41402623f8cf0933a19efdd7bbd9cd81)
+++ b/js/buoyancy-basics/model/BuoyancyBasicsExploreModel.ts	(date 1716410692622)
@@ -31,7 +31,7 @@
   public readonly modeProperty: Property<TwoBlockMode>;
   public readonly primaryMass: Cube;
   public readonly secondaryMass: Cube;
-  public readonly densityExpandedProperty = new BooleanProperty( false );
+  // public readonly densityExpandedProperty = new BooleanProperty( false );
   public readonly percentageSubmergedExpandedProperty = new BooleanProperty( false );
   public readonly poolScaleHeightProperty: NumberProperty;
   public readonly poolScale: Scale;
@@ -108,7 +108,7 @@
     this.primaryMass.reset();
     this.secondaryMass.reset();
 
-    this.densityExpandedProperty.reset();
+    // this.densityExpandedProperty.reset();
 
     super.reset();
 
Index: js/common/view/ScaleReadoutNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/ScaleReadoutNode.ts b/js/common/view/ScaleReadoutNode.ts
--- a/js/common/view/ScaleReadoutNode.ts	(revision 3978f23c41402623f8cf0933a19efdd7bbd9cd81)
+++ b/js/common/view/ScaleReadoutNode.ts	(date 1716474019843)
@@ -19,6 +19,57 @@
 import Gravity from '../model/Gravity.js';
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
 import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
+import Property from '../../../../axon/js/Property.js';
+import InterpolatedProperty from '../model/InterpolatedProperty.js';
+
+class AverageProperty extends Property<number> {
+  public constructor( sourceProperty: TReadOnlyProperty<number> ) {
+
+    // let val = sourceProperty.value;
+    const history: number[] = [];
+
+    super( sourceProperty.value );
+
+    sourceProperty.link( value => {
+      history.push( value );
+      if ( history.length > 60 ) {
+        history.shift();
+      }
+
+      // val = InterpolatedProperty.interpolateNumber( val, value, 0.01 );
+      // this.value = val;
+
+      const max = Math.max( ...history );
+      const min = Math.min( ...history );
+
+
+      const number = Math.abs( max - min );
+      console.log( number );
+      if ( number > 0.1 && false) {
+        this.value = value;
+      }
+      else {
+        this.value = _.mean( history );
+        // this.value = getMedian( history );
+      }
+
+
+    } );
+
+
+  }
+}
+
+const getMedian = ( values: number[] ) => {
+  values.sort( ( a, b ) => a - b );
+  const half = Math.floor( values.length / 2 );
+
+  if ( values.length % 2 ) {
+    return values[ half ];
+  }
+
+  return ( values[ half - 1 ] + values[ half ] ) / 2.0;
+};
 
 export default class ScaleReadoutNode extends Node {
 
@@ -31,8 +82,10 @@
       pickable: false
     } );
 
+    const average = new AverageProperty( mass.scaleForceInterpolatedProperty );
+
     this.stringProperty = new DerivedProperty( [
-      mass.scaleForceInterpolatedProperty,
+      average,
       gravityProperty,
       DensityBuoyancyCommonStrings.newtonsPatternStringProperty,
       DensityBuoyancyCommonStrings.kilogramsPatternStringProperty

[samreid]

From the design meeting, we would like to investigate a filtering, like the Kalman filter.

[samreid]

I tried a variety of noise settings for the Kalman filter but can't get it to stop flickering either:

Subject: [PATCH] Pass launchButtonEnabledProperty directly to the launchButton.enabledProperty, see https://github.com/phetsims/projectile-data-lab/issues/141
---
Index: js/common/view/ScaleReadoutNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/ScaleReadoutNode.ts b/js/common/view/ScaleReadoutNode.ts
--- a/js/common/view/ScaleReadoutNode.ts	(revision 825de869172cc4f7f01f8ea8a9d9f542d93c0492)
+++ b/js/common/view/ScaleReadoutNode.ts	(date 1717036007450)
@@ -19,6 +19,128 @@
 import Gravity from '../model/Gravity.js';
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
 import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
+import Property from '../../../../axon/js/Property.js';
+
+/**
+ * KalmanFilter
+ * @class
+ * @author Wouter Bulten
+ * @see {@link http://github.com/wouterbulten/kalmanjs}
+ * @version Version: 1.0.0-beta
+ * @copyright Copyright 2015-2018 Wouter Bulten
+ * @license MIT License
+ * @preserve
+ */
+class KalmanFilter {
+
+  /**
+   * Create 1-dimensional kalman filter
+   * @param  {Number} options.R Process noise
+   * @param  {Number} options.Q Measurement noise
+   * @param  {Number} options.A State vector
+   * @param  {Number} options.B Control vector
+   * @param  {Number} options.C Measurement vector
+   * @return {KalmanFilter}
+   */
+  constructor( { R = 1, Q = 1, A = 1, B = 0, C = 1 } = {} ) {
+
+    this.R = R; // noise power desirable
+    this.Q = Q; // noise power estimated
+
+    this.A = A;
+    this.C = C;
+    this.B = B;
+    this.cov = NaN;
+    this.x = NaN; // estimated signal without noise
+  }
+
+  /**
+   * Filter a new value
+   * @param  {Number} z Measurement
+   * @param  {Number} u Control
+   * @return {Number}
+   */
+  filter( z, u = 0 ) {
+
+    if ( isNaN( this.x ) ) {
+      this.x = ( 1 / this.C ) * z;
+      this.cov = ( 1 / this.C ) * this.Q * ( 1 / this.C );
+    }
+    else {
+
+      // Compute prediction
+      const predX = this.predict( u );
+      const predCov = this.uncertainty();
+
+      // Kalman gain
+      const K = predCov * this.C * ( 1 / ( ( this.C * predCov * this.C ) + this.Q ) );
+
+      // Correction
+      this.x = predX + K * ( z - ( this.C * predX ) );
+      this.cov = predCov - ( K * this.C * predCov );
+    }
+
+    return this.x;
+  }
+
+  /**
+   * Predict next value
+   * @param  {Number} [u] Control
+   * @return {Number}
+   */
+  predict( u = 0 ) {
+    return ( this.A * this.x ) + ( this.B * u );
+  }
+
+  /**
+   * Return uncertainty of filter
+   * @return {Number}
+   */
+  uncertainty() {
+    return ( ( this.A * this.cov ) * this.A ) + this.R;
+  }
+
+  /**
+   * Return the last filtered measurement
+   * @return {Number}
+   */
+  lastMeasurement() {
+    return this.x;
+  }
+
+  /**
+   * Set measurement noise Q
+   * @param {Number} noise
+   */
+  setMeasurementNoise( noise ) {
+    this.Q = noise;
+  }
+
+  /**
+   * Set the process noise R
+   * @param {Number} noise
+   */
+  setProcessNoise( noise ) {
+    this.R = noise;
+  }
+}
+
+class AverageProperty extends Property<number> {
+  public constructor( sourceProperty: TReadOnlyProperty<number> ) {
+
+    const k = new KalmanFilter( { R: 100, Q: 20 } );
+
+    super( sourceProperty.value );
+
+    sourceProperty.link( value => {
+
+      const result = k.filter( value );
+
+      this.value = result;
+      console.log( result );
+    } );
+  }
+}
 
 export default class ScaleReadoutNode extends Node {
 
@@ -31,8 +153,10 @@
       pickable: false
     } );
 
+    const average = new AverageProperty( mass.scaleForceInterpolatedProperty );
+
     this.stringProperty = new DerivedProperty( [
-      mass.scaleForceInterpolatedProperty,
+      average,
       gravityProperty,
       DensityBuoyancyCommonStrings.newtonsPatternStringProperty,
       DensityBuoyancyCommonStrings.kilogramsPatternStringProperty

[samreid]

This patch is very similar to @AgustinVallejo pull request. It helps in many cases but you can still trigger fluctuations. We recalled there was another idea from @kathy-phet to show only 1 decimal place when values are over 10, which will also help, but we don't see that in another issue.

Subject: [PATCH] Pass launchButtonEnabledProperty directly to the launchButton.enabledProperty, see https://github.com/phetsims/projectile-data-lab/issues/141
---
Index: js/common/view/ScaleReadoutNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/ScaleReadoutNode.ts b/js/common/view/ScaleReadoutNode.ts
--- a/js/common/view/ScaleReadoutNode.ts	(revision 825de869172cc4f7f01f8ea8a9d9f542d93c0492)
+++ b/js/common/view/ScaleReadoutNode.ts	(date 1717039782790)
@@ -19,6 +19,32 @@
 import Gravity from '../model/Gravity.js';
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
 import DerivedProperty from '../../../../axon/js/DerivedProperty.js';
+import Property from '../../../../axon/js/Property.js';
+
+class AverageProperty extends Property<number> {
+  public constructor( sourceProperty: TReadOnlyProperty<number> ) {
+
+    super( sourceProperty.value );
+
+    sourceProperty.link( value => {
+
+      const oldValue = this.value;
+      const newValue = value;
+
+      // choose the blendAmount based on the distance between values
+      // This adds a hysteresis effect to the readout, which reduces flickering inherent to the model.
+      const MIN_BLEND = 0.1; // When close, blend with the old value more
+      const MAX_BLEND = 1; // When far apart, take the new value completely
+      const blendAmount = Utils.clamp( Utils.linear( 0, 1, MIN_BLEND, MAX_BLEND, Math.abs( newValue - oldValue ) ), MIN_BLEND, MAX_BLEND );
+
+      // blend
+      const result = blendAmount * newValue + ( 1 - blendAmount ) * oldValue;
+
+      this.value = result;
+      // console.log( result );
+    } );
+  }
+}
 
 export default class ScaleReadoutNode extends Node {
 
@@ -31,8 +57,10 @@
       pickable: false
     } );
 
+    const averageProperty = new AverageProperty( mass.scaleForceInterpolatedProperty );
+
     this.stringProperty = new DerivedProperty( [
-      mass.scaleForceInterpolatedProperty,
+      averageProperty,
       gravityProperty,
       DensityBuoyancyCommonStrings.newtonsPatternStringProperty,
       DensityBuoyancyCommonStrings.kilogramsPatternStringProperty

[samreid]

@AgustinVallejo and I reviewed the Kalman filter and agreed it doesn't seem like it will work for this context. Instead, we continued the blending approach, which has a good behavior and works well in this context. We adjusted the coefficients so that far away values favor the new value and there is more blending when the values are close. There will still be edge cases where there is flickering due to the inherent noisy nature of the model. This is ready for @DianaTavares to review. Please close if all is well.

UPDATE: Also the change in #167 reduces the amount of flickering in the sim.

[AgustinVallejo]

This issue is entangled with #140, if this one is ready for closing, we should check that one as well. As the fix encompasses both visual flickers.

[DianaTavares]

I see a little fluctuation only in the bottle:

and in boat in Tantalum (big density)

But I think that is good enough to go!! Then I am closing this issue.

[zepumph]

I believe these blended Properties are creating memory leaks, since the views can be created and disposed quite often, depending on what masses are being shown. That said I couldn't find any usages of us recreating mass views, even in PhET-iO. Perhaps that is because all masses are getting statically allocated on startup now.