phetsims / fourier-making-waves

"Fourier: Making Waves" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 3 forks source link

CT: unsupported equationForm #238

Closed samreid closed 11 months ago

samreid commented 11 months ago

CT Showed this error today in the 9/21/2023 5:54:46pm column:

fourier-making-waves : fuzz : unbuilt : listenerOrderRandom
http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/fourier-making-waves/fourier-making-waves_en.html?continuousTest=%7B%22test%22%3A%5B%22fourier-making-waves%22%2C%22fuzz%22%2C%22unbuilt%22%2C%22listenerOrderRandom%22%5D%2C%22snapshotName%22%3A%22snapshot-1695340486384%22%2C%22timestamp%22%3A1695346878978%7D&brand=phet&ea&fuzz&listenerOrder=random
Query: brand=phet&ea&fuzz&listenerOrder=random
Uncaught Error: Assertion failed: unsupported equationForm: PERIOD

Full details:

``` fourier-making-waves : fuzz : unbuilt : listenerOrderRandom http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/fourier-making-waves/fourier-making-waves_en.html?continuousTest=%7B%22test%22%3A%5B%22fourier-making-waves%22%2C%22fuzz%22%2C%22unbuilt%22%2C%22listenerOrderRandom%22%5D%2C%22snapshotName%22%3A%22snapshot-1695340486384%22%2C%22timestamp%22%3A1695346878978%7D&brand=phet&ea&fuzz&listenerOrder=random Query: brand=phet&ea&fuzz&listenerOrder=random Uncaught Error: Assertion failed: unsupported equationForm: PERIOD Error: Assertion failed: unsupported equationForm: PERIOD at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/assert/js/assert.js:28:13) at assert (EquationMarkup.ts:96:12) at getSpaceMarkup (EquationMarkup.ts:56:15) at getSpecificFormMarkup (EquationMarkup.ts:47:26) at getGeneralFormMarkup (DiscreteSumEquationNode.ts:82:42) at callback (Multilink.ts:111:10) at listener (TinyEmitter.ts:123:8) at emit (ReadOnlyProperty.ts:315:22) at _notifyListeners (ReadOnlyProperty.ts:262:13) at unguardedSet (ReadOnlyProperty.ts:246:11) [URL] http://128.138.93.172/continuous-testing/aqua/html/sim-test.html?url=..%2F..%2Fct-snapshots%2F1695340486384%2Ffourier-making-waves%2Ffourier-making-waves_en.html&simQueryParameters=brand%3Dphet%26ea%26fuzz%26listenerOrder%3Drandom&testInfo=%7B%22test%22%3A%5B%22fourier-making-waves%22%2C%22fuzz%22%2C%22unbuilt%22%2C%22listenerOrderRandom%22%5D%2C%22snapshotName%22%3A%22snapshot-1695340486384%22%2C%22timestamp%22%3A1695346878978%7D [NAVIGATED] http://128.138.93.172/continuous-testing/aqua/html/sim-test.html?url=..%2F..%2Fct-snapshots%2F1695340486384%2Ffourier-making-waves%2Ffourier-making-waves_en.html&simQueryParameters=brand%3Dphet%26ea%26fuzz%26listenerOrder%3Drandom&testInfo=%7B%22test%22%3A%5B%22fourier-making-waves%22%2C%22fuzz%22%2C%22unbuilt%22%2C%22listenerOrderRandom%22%5D%2C%22snapshotName%22%3A%22snapshot-1695340486384%22%2C%22timestamp%22%3A1695346878978%7D [NAVIGATED] about:blank [NAVIGATED] http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/fourier-making-waves/fourier-making-waves_en.html?continuousTest=%7B%22test%22%3A%5B%22fourier-making-waves%22%2C%22fuzz%22%2C%22unbuilt%22%2C%22listenerOrderRandom%22%5D%2C%22snapshotName%22%3A%22snapshot-1695340486384%22%2C%22timestamp%22%3A1695346878978%7D&brand=phet&ea&fuzz&listenerOrder=random [CONSOLE] enabling assert [CONSOLE] listenerOrder random seed: 503360 [CONSOLE] continuous-test-load [CONSOLE] Assertion failed: unsupported equationForm: PERIOD [PAGE ERROR] Error: Error: Assertion failed: unsupported equationForm: PERIOD at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/assert/js/assert.js:28:13) at getSpaceMarkup (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/fourier-making-waves/js/common/view/EquationMarkup.js:85:13) at Object.getSpecificFormMarkup (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/fourier-making-waves/js/common/view/EquationMarkup.js:52:16) at Object.getGeneralFormMarkup (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/fourier-making-waves/js/common/view/EquationMarkup.js:44:27) at http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/fourier-making-waves/js/discrete/view/DiscreteSumEquationNode.js:50:41 at listener (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/Multilink.js:50:11) at TinyProperty.emit (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/TinyEmitter.js:96:9) at EnumerationProperty._notifyListeners (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:239:23) at EnumerationProperty.unguardedSet (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:188:14) at EnumerationProperty.set (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:173:12) [CONSOLE] continuous-test-error [CONSOLE] Assertion failed: unsupported equationForm: PERIOD [PAGE ERROR] Error: Error: Assertion failed: unsupported equationForm: PERIOD at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/assert/js/assert.js:28:13) at getSpaceMarkup (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/fourier-making-waves/js/common/view/EquationMarkup.js:85:13) at Object.getSpecificFormMarkup (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/fourier-making-waves/js/common/view/EquationMarkup.js:52:16) at http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/fourier-making-waves/js/discrete/view/ExpandedFormDialog.js:58:45 at listener (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/Multilink.js:50:11) at TinyProperty.emit (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/TinyEmitter.js:96:9) at NumberProperty._notifyListeners (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:239:23) at NumberProperty.unguardedSet (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:188:14) at NumberProperty.set (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:173:12) at NumberProperty.set (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/Property.js:58:11) [CONSOLE] continuous-test-error [CONSOLE] Assertion failed: unsupported equationForm: PERIOD [PAGE ERROR] Error: Error: Assertion failed: unsupported equationForm: PERIOD at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/assert/js/assert.js:28:13) at getSpaceMarkup (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/fourier-making-waves/js/common/view/EquationMarkup.js:85:13) at Object.getSpecificFormMarkup (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/fourier-making-waves/js/common/view/EquationMarkup.js:52:16) at http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/fourier-making-waves/js/discrete/view/ExpandedFormDialog.js:58:45 at listener (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/Multilink.js:50:11) at TinyProperty.emit (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/TinyEmitter.js:96:9) at DerivedProperty._notifyListeners (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:239:23) at DerivedProperty.unguardedSet (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:188:14) at DerivedProperty.set (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:173:12) at DerivedProperty.getDerivedPropertyListener (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/DerivedProperty.js:111:13) [CONSOLE] continuous-test-error [CONSOLE] Assertion failed: reentry detected, value=0, oldValue=0.65 [PAGE ERROR] Error: Error: Assertion failed: reentry detected, value=0, oldValue=0.65 at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/assert/js/assert.js:28:13) at NumberProperty._notifyListeners (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:237:15) at NumberProperty.unguardedSet (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:188:14) at NumberProperty.set (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:173:12) at NumberProperty.set (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/Property.js:58:11) at handleTrackEvent (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/sun/js/SliderTrack.js:84:21) at DragListener.drag [as _dragListener] (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/sun/js/SliderTrack.js:104:9) at DragListener.drag (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/scenery/js/listeners/PressListener.js:341:10) at DragListener._dragAction.PhetioAction.parameters.name (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/scenery/js/listeners/DragListener.js:166:36) at PhetioAction.execute (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/tandem/js/PhetioAction.js:132:17) [CONSOLE] continuous-test-error [CONSOLE] Assertion failed: reentry detected, value=1,0,0,0,0.6,0,0,0,0,0,0, oldValue=1,0,0,0,0,0,0,0,0,0.65,0 [PAGE ERROR] Error: Error: Assertion failed: reentry detected, value=1,0,0,0,0.6,0,0,0,0,0,0, oldValue=1,0,0,0,0,0,0,0,0,0.65,0 at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/assert/js/assert.js:28:13) at DerivedProperty._notifyListeners (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:237:15) at DerivedProperty.unguardedSet (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:188:14) at DerivedProperty.set (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:173:12) at DerivedProperty.getDerivedPropertyListener (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/DerivedProperty.js:111:13) at TinyProperty.emit (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/TinyEmitter.js:96:9) at NumberProperty._notifyListeners (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:239:23) at NumberProperty.unguardedSet (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:188:14) at NumberProperty.set (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/ReadOnlyProperty.js:173:12) at NumberProperty.set (http://128.138.93.172/continuous-testing/ct-snapshots/1695340486384/chipper/dist/js/axon/js/Property.js:58:11) [CONSOLE] continuous-test-error id: "Sparky Node Puppeteer" Snapshot from 9/21/2023, 5:54:46 PM ```
pixelzoom commented 11 months ago

The above stack trace indicates that the problem occurs in DiscreteSumEquationNode.ts.

Here's the failing assertion in EquationMarkup.ts. getSpaceMarkup is being called with an EquationForm that is not appropriate for the "space" domain.

function getSpaceMarkup( seriesType: SeriesType, equationForm: EquationForm, order: Order, amplitude: Amplitude ): string {
  assert && assert( [ EquationForm.HIDDEN, EquationForm.WAVELENGTH, EquationForm.SPATIAL_WAVE_NUMBER, EquationForm.MODE ].includes( equationForm ),
    `unsupported equationForm: ${equationForm}` );
pixelzoom commented 11 months ago

Looking back through CT history, here are a couple more examples where getTimeMarkup fails:

This one originates in HarmonicsEquationNode.ts:

fourier-making-waves : fuzz : unbuilt : listenerOrderRandom
http://128.138.93.172/continuous-testing/ct-snapshots/1695568347453/fourier-making-waves/fourier-making-waves_en.html?continuousTest=%7B%22test%22%3A%5B%22fourier-making-waves%22%2C%22fuzz%22%2C%22unbuilt%22%2C%22listenerOrderRandom%22%5D%2C%22snapshotName%22%3A%22snapshot-1695568347453%22%2C%22timestamp%22%3A1695573706050%7D&brand=phet&ea&fuzz&listenerOrder=random
Query: brand=phet&ea&fuzz&listenerOrder=random
Uncaught Error: Assertion failed: unsupported equationForm: WAVELENGTH
Error: Assertion failed: unsupported equationForm: WAVELENGTH
at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1695568347453/assert/js/assert.js:28:13)
at assert (EquationMarkup.ts:155:12)
at getSpaceAndTimeMarkup (EquationMarkup.ts:62:15)
at getSpecificFormMarkup (EquationMarkup.ts:47:26)
at getGeneralFormMarkup (HarmonicsEquationNode.ts:43:41)
...

This one originates in ExpandedFormDialog.ts:

fourier-making-waves : fuzz : unbuilt : listenerOrderRandom
http://128.138.93.172/continuous-testing/ct-snapshots/1695576926864/fourier-making-waves/fourier-making-waves_en.html?continuousTest=%7B%22test%22%3A%5B%22fourier-making-waves%22%2C%22fuzz%22%2C%22unbuilt%22%2C%22listenerOrderRandom%22%5D%2C%22snapshotName%22%3A%22snapshot-1695576926864%22%2C%22timestamp%22%3A1695592020686%7D&brand=phet&ea&fuzz&listenerOrder=random
Query: brand=phet&ea&fuzz&listenerOrder=random
Uncaught Error: Assertion failed: unsupported equationForm: SPATIAL_WAVE_NUMBER
Error: Assertion failed: unsupported equationForm: SPATIAL_WAVE_NUMBER
at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1695576926864/assert/js/assert.js:28:13)
at assert (EquationMarkup.ts:124:12)
at getTimeMarkup (EquationMarkup.ts:59:15)
at getSpecificFormMarkup (ExpandedFormDialog.ts:76:46)
...
pixelzoom commented 11 months ago

Here are the 3 places where there's an ordering dependency that causes getSpaceMarkup to fail. When domainProperty changes before equationFormProperty, it's possible that the assertion in EquationMarkup getSpaceMarkup will be violated, because equationFormProperty will be stale (in transition).

DiscreteSumEquationNode.ts:

    const multilink = new Multilink(
      [ domainProperty, seriesTypeProperty, equationFormProperty ],
      ( domain, seriesType, equationForm ) => {
...
        rightNode.string = EquationMarkup.getGeneralFormMarkup( domain, seriesType, equationForm );
...
      } );

HarmonicsEquationNode.ts:

    Multilink.multilink(
      [ domainProperty, seriesTypeProperty, equationFormProperty ],
      ( domain, seriesType, equationForm ) => {
        richText.string = EquationMarkup.getGeneralFormMarkup( domain, seriesType, equationForm );
      }
    );

ExpandedFormDialog.ts:

    Multilink.multilink(
      [ fourierSeries.numberOfHarmonicsProperty, fourierSeries.amplitudesProperty, domainProperty, seriesTypeProperty, equationFormProperty ],
      ( numberOfHarmonics, amplitudes, domain, seriesType, equationForm ) => {
...
          expandedSumMarkup += EquationMarkup.getSpecificFormMarkup( domain, seriesType, equationForm, order, amplitude );
...
      } );
pixelzoom commented 11 months ago

To manually reproduce the error that originates in DiscreteSumEquationNode.ts:

  1. Run the sim with ?brand=phet&ea&debugger&listenerOrder=random(548659)
  2. Go to the Discrete screen
  3. Change "Equation" combo box to "λ".
  4. Change "Function of" combo box to "time (t)". The assertion in getSpaceMarkup will fail.
pixelzoom commented 11 months ago

DiscreteModel adjusts equationFormProperty to match domainProperty:

    // Ensure that the math form is appropriate for the Domain. EquationForm.MODE is supported for all Domain values.
    this.domainProperty.link( () => {
      if ( this.equationFormProperty.value !== EquationForm.MODE ) {
        this.equationFormProperty.value = EquationForm.HIDDEN;
      }
    } );

So the above commit handles the intermediate state by treating it like EquationForm.HIDDEN.

I'll leave this open until I confirm that CT is happy.

FYI, I spent 1.5 hours on this. I'm not sure that it was worth it.

pixelzoom commented 11 months ago

There has been no evidence of this error in CT, so closing.