Speed histogram displays no data in the Standard wrapper.

[pixelzoom]

Reported by @arouinfar and @Nancy-Salpepi.

To reproduce:

1. Run the sim in Studio.
2. Go to the Energy screen.
3. Add particles to the container.
4. Wait for the histograms to display data.
5. Pause the sim. It will look something like the first screenshot below.
6. Press the "Test" button in Studio to launch the Standard wrapper. It will look something like the second screenshot below. Note that the Speed histogram is displaying no data.
7. Press step in the Standard wrapper, and the Speed histogram will display data.
8. Press Reset All in the Standard wrapper and the Speed histogram will again display no data.

Note that something similar happens in the State wrapper with Set-State Rate set to zero. Updating of the histograms lags in the Downstream sim.

[pixelzoom]

The state schema for HistogramModelIO involves ReferenceArrayIO, which (as I understand it) is relatively new and not widely used. To verify that it's not causing this problem, I switched to ArrayIO and (rather than relying on the default serialization functions) explicilty defined toStateObject and applyState -- see the patch below. This does not resolve the problem, but rules out ReferenceArrayIO and the default serialization functions.

patch

Subject: [PATCH] initialize rightPanels position before creating dragBoundsProperty for compassNode, https://github.com/phetsims/faradays-electromagnetic-lab/issues/183
---
Index: js/energy/model/HistogramsModel.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/energy/model/HistogramsModel.ts b/js/energy/model/HistogramsModel.ts
--- a/js/energy/model/HistogramsModel.ts	(revision d7277601a6bc5ad6e30705329897bf243878c74b)
+++ b/js/energy/model/HistogramsModel.ts	(date 1718211876094)
@@ -21,7 +21,6 @@
 import gasProperties from '../../gasProperties.js';
 import IOType from '../../../../tandem/js/types/IOType.js';
 import isSettingPhetioStateProperty from '../../../../tandem/js/isSettingPhetioStateProperty.js';
-import ReferenceArrayIO from '../../../../tandem/js/types/ReferenceArrayIO.js';
 
 // Describes the properties of the histograms at a specific zoom level.
 type ZoomLevel = {
@@ -48,10 +47,10 @@
 const STATE_SCHEMA = {
   dtAccumulator: NumberIO,
   numberOfSamples: NumberIO,
-  heavySpeedCumulativeBinCounts: ReferenceArrayIO( NumberIO ),
-  lightSpeedCumulativeBinCounts: ReferenceArrayIO( NumberIO ),
-  heavyKineticEnergyCumulativeBinCounts: ReferenceArrayIO( NumberIO ),
-  lightKineticEnergyCumulativeBinCounts: ReferenceArrayIO( NumberIO )
+  heavySpeedCumulativeBinCounts: ArrayIO( NumberIO ),
+  lightSpeedCumulativeBinCounts: ArrayIO( NumberIO ),
+  heavyKineticEnergyCumulativeBinCounts: ArrayIO( NumberIO ),
+  lightKineticEnergyCumulativeBinCounts: ArrayIO( NumberIO )
 };
 
 export default class HistogramsModel extends PhetioObject {
@@ -325,6 +324,41 @@
     this.clearSamples();
   }
 
+  /**
+   * Serializes this instance of HistogramsModel.
+   */
+  private toStateObject(): HistogramsModelStateObject {
+    return {
+      dtAccumulator: this.dtAccumulator,
+      numberOfSamples: this.numberOfSamples,
+      heavySpeedCumulativeBinCounts: ArrayIO( NumberIO ).toStateObject( this.heavySpeedCumulativeBinCounts ),
+      lightSpeedCumulativeBinCounts: ArrayIO( NumberIO ).toStateObject( this.lightSpeedCumulativeBinCounts ),
+      heavyKineticEnergyCumulativeBinCounts: ArrayIO( NumberIO ).toStateObject( this.heavyKineticEnergyCumulativeBinCounts ),
+      lightKineticEnergyCumulativeBinCounts: ArrayIO( NumberIO ).toStateObject( this.lightKineticEnergyCumulativeBinCounts )
+    };
+  }
+
+  /**
+   * Deserializes an instance of HistogramsModel.
+   */
+  private static applyState( histogramsModel: HistogramsModel, stateObject: HistogramsModelStateObject ): void {
+
+    histogramsModel.dtAccumulator = stateObject.dtAccumulator;
+    histogramsModel.numberOfSamples = stateObject.numberOfSamples;
+
+    histogramsModel.heavySpeedCumulativeBinCounts.length = 0;
+    histogramsModel.heavySpeedCumulativeBinCounts.push( ...stateObject.heavySpeedCumulativeBinCounts );
+
+    histogramsModel.lightSpeedCumulativeBinCounts.length = 0;
+    histogramsModel.lightSpeedCumulativeBinCounts.push( ...stateObject.lightSpeedCumulativeBinCounts );
+
+    histogramsModel.heavyKineticEnergyCumulativeBinCounts.length = 0;
+    histogramsModel.heavyKineticEnergyCumulativeBinCounts.push( ...stateObject.heavyKineticEnergyCumulativeBinCounts );
+
+    histogramsModel.lightKineticEnergyCumulativeBinCounts.length = 0;
+    histogramsModel.lightKineticEnergyCumulativeBinCounts.push( ...stateObject.lightKineticEnergyCumulativeBinCounts );
+  }
+
   /**
    * HistogramModelIO handles serialization of data that supports derivation of speed and kinetic energy histograms.
    * It implements reference-type serialization, as described in
@@ -334,9 +368,11 @@
     valueType: HistogramsModel,
     stateSchema: STATE_SCHEMA,
     documentation: 'PhET-iO Type that supports sampling of the Speed and Kinetic Energy of the particle system. ' +
-                   'All fields in this type are for internal use only.'
+                   'All fields in this type are for internal use only.',
     // toStateObject: Use the default, which is derived from stateSchema.
+    toStateObject: histogramsModel => histogramsModel.toStateObject(),
     // applyState: Use the default, which is derived from stateSchema.
+    applyState: HistogramsModel.applyState
   } );
 }
 

[pixelzoom]

The problem is specific to totalSpeedBinCountsProperty. heavySpeedBinCountsProperty and lightSpeedBinCountsProperty are properly restored.

To demonstrate... If I check both checkboxes, like this:

... then the Standard Wrapper looks like this:

[pixelzoom]

Adding this to EnergyModel.ts:

    this.histogramsModel.totalSpeedBinCountsProperty.link( totalSpeedBinCounts => {
      console.log( `heavySpeedBinCounts = ${this.histogramsModel.heavySpeedBinCountsProperty.value}` );
      console.log( `lightSpeedBinCounts = ${this.histogramsModel.lightSpeedBinCountsProperty.value}` );
      console.log( `totalSpeedBinCounts = ${totalSpeedBinCounts}` );
      console.log( '------' );
    } );

Running the Standard wrapper paused with 50 heavy particles, I see this in the console:

EnergyModel.ts:42 heavySpeedBinCounts = 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
EnergyModel.ts:43 lightSpeedBinCounts = 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
EnergyModel.ts:44 totalSpeedBinCounts = 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
EnergyModel.ts:45 ------
EnergyModel.ts:42 heavySpeedBinCounts = 5.708333333333333,12.958333333333334,13.333333333333334,7.333333333333333,4.666666666666667,6,0,0,0,0,0,0,0,0,0,0,0,0,0
EnergyModel.ts:43 lightSpeedBinCounts = 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
EnergyModel.ts:44 totalSpeedBinCounts = 5.708333333333333,12.958333333333334,13.333333333333334,7.333333333333333,4.666666666666667,6,0,0,0,0,0,0,0,0,0,0,0,0,0
EnergyModel.ts:45 ------

These values look correct. The first set of output is when the sim starts, the second set is when the initial state of the Standard Wrapper is restored.

[pixelzoom]

Collaping and expanding the Speed accordion box in the Standard Wrapper will cause the histogram to update. So I suspect that there is a problem related to histogramNode.updateEnabledProperty, which is a performance optimization that prevents the histogram from updating while the accordion box is collapsed.

[pixelzoom]

Fixed in the above commits. There was a performance optimization (based on a pesky Emitter) that was resulting in failure to update the Speed "total" plot when restoring state. No idea how the KE "total" plot was getting updated properly, perhaps because it was collapsed by default. I replaced with a superior optimization based on Property. This fixes the problem observed in the Standard wrapper, and the similar "lag" problem in the State wrapper.

@arouinfar @Nancy-Salpepi please review. The last person to review may close. Note that this required major changes to the histograms, so 👀 for regressions.

[arouinfar]

Good news, the histograms appear to be stateful. Bad news, #262 was introduced. Seems like we should hold off on further testing until it's addressed.

[pixelzoom]

#262 has been addressed, so you may resume review.

[arouinfar]

Thanks @pixelzoom. The histograms are looking good on main in the state wrapper and Studio. I'll leave this open for @Nancy-Salpepi to review too.

[Nancy-Salpepi]

Histograms look good to me too! Closing.