Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error: Assertion failed: unexpected emfProperty.value #149

Closed
samreid opened this issue Mar 28, 2024 · 8 comments
Closed

Error: Assertion failed: unexpected emfProperty.value #149

samreid opened this issue Mar 28, 2024 · 8 comments

Comments

@samreid
Copy link
Member

samreid commented Mar 28, 2024

Discovered while working on #103, while testing the state wrapper on macbook air m1 chrome, I believe I changed to the AC source right before this happened:

PhetioClient.ts:2052 Uncaught (in promise) Error: Assertion failed: unexpected emf: -3252.5441996147506
    at window.assertions.assertFunction (assert.js:28:13)
    at Transformer.ts:77:17
    at TinyProperty.notifyLoop (TinyEmitter.ts:213:7)
    at TinyProperty.emit (TinyEmitter.ts:185:18)
    at Property._notifyListeners (ReadOnlyProperty.ts:336:23)
    at PhaseCallback.listener (ReadOnlyProperty.ts:384:47)
    at PropertyStateHandler.attemptToApplyPhases (PropertyStateHandler.ts:273:41)
    at PropertyStateHandler.undeferAndNotifyProperties (PropertyStateHandler.ts:193:12)
    at PropertyStateHandler.ts:88:12
    at TinyEmitter.notifyLoop (TinyEmitter.ts:213:7)
image

I confirmed that these steps reproduce the problem:

  1. Launch the simulation
  2. Open the "Transformer" screen
  3. Press the AC current source radio button
@samreid samreid added type:bug Something isn't working dev:code-review labels Mar 28, 2024
@samreid
Copy link
Member Author

samreid commented Mar 28, 2024

This is also occurring on CT

@pixelzoom pixelzoom changed the title Error: Assertion failed: unexpected emf Error: Assertion failed: unexpected emfProperty.value Mar 29, 2024
pixelzoom added a commit that referenced this issue Mar 29, 2024
@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 29, 2024

Assertion message changed in the above commit, and updated in the issue title.

Reproduced in my working copy with http://localhost:8080/phet-io-wrappers/state/?sim=faradays-electromagnetic-lab&locales=*&phetioWrapperDebug=true&fuzz&phetioDebug=true&audio=disabled

This is related to #124, the workaround for zeroing out EMF when switching power supplies. The relevant code in Transformer.ts is:

    // Workaround for https://github.com/phetsims/faradays-electromagnetic-lab/issues/92, to ignore the EMF induced
    // by switching power supplies. Step the pickup coil twice, so that there is effectively no induced EMF.
    this.electromagnet.currentSourceProperty.lazyLink( () => {
      this.pickupCoil.step( ConstantDtClock.DT ); // EMF may be induced by changing from oldCurrentSource to newCurrentSource.
      this.pickupCoil.step( ConstantDtClock.DT ); // No EMF is induced because there is no flux change in newCurrentSource.
      assert && assert( this.pickupCoil.emfProperty.value === 0, `unexpected emfProperty.value: ${this.pickupCoil.emfProperty.value}` );
    } );

I'm glad I added this assertion, because something is clearly not right when state is restored. Maybe I need to add anisSettingPhetioStateProperty guard to skip this code when restoring PhET-iO state.

@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 29, 2024

Maybe I need to add anisSettingPhetioStateProperty guard to skip this code when restoring PhET-iO state.

That's what I did in f3773ce. It seems to resolve the problem in my working copy, and we'll see what CT has to say. But I don't understand why it's necessary, or why it's even effective. And I'm concerned that it may result in incorrect behavior when switching power supplies.

@pixelzoom
Copy link
Contributor

pixelzoom commented Apr 3, 2024

The code identified in #149 (comment) has been replaced by a new implementation for avoiding induced EMF when changing power supplies -- see #124. So I've removed the isSettingPhetioStateProperty guard, and we'll see if CT has a problem with the new implementation.

@pixelzoom
Copy link
Contributor

Still occurring in CT, most recently:

aradays-electromagnetic-lab : phet-io-state-fuzz : unbuilt
http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/phet-io-wrappers/state/?sim=faradays-electromagnetic-lab&locales=*&phetioWrapperDebug=true&fuzz&phetioDebug=true&wrapperContinuousTest=%7B%22test%22%3A%5B%22faradays-electromagnetic-lab%22%2C%22phet-io-state-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1712311463115%22%2C%22timestamp%22%3A1712329940914%7D
Uncaught Error: Assertion failed: mismatched index, possible start/end mismatch. Likely this is downstream of another error, try pausing on caught exceptions.
Error: Assertion failed: mismatched index, possible start/end mismatch. Likely this is downstream of another error, try pausing on caught exceptions.
at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/assert/js/assert.js:28:13)
at assert (dataStream.ts:299:14)
at end (PhetioObject.ts:514:29)
at phetioEndEvent (phetioCommandProcessor.ts:254:17)
[URL] http://128.138.93.172/continuous-testing/aqua/html/wrapper-test.html?url=..%2F..%2Fct-snapshots%2F1712311463115%2Fphet-io-wrappers%2Fstate%2F%3Fsim%3Dfaradays-electromagnetic-lab%26locales%3D*%26phetioWrapperDebug%3Dtrue%26fuzz%26phetioDebug%3Dtrue&testInfo=%7B%22test%22%3A%5B%22faradays-electromagnetic-lab%22%2C%22phet-io-state-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1712311463115%22%2C%22timestamp%22%3A1712329940914%7D
[NAVIGATED] http://128.138.93.172/continuous-testing/aqua/html/wrapper-test.html?url=..%2F..%2Fct-snapshots%2F1712311463115%2Fphet-io-wrappers%2Fstate%2F%3Fsim%3Dfaradays-electromagnetic-lab%26locales%3D*%26phetioWrapperDebug%3Dtrue%26fuzz%26phetioDebug%3Dtrue&testInfo=%7B%22test%22%3A%5B%22faradays-electromagnetic-lab%22%2C%22phet-io-state-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1712311463115%22%2C%22timestamp%22%3A1712329940914%7D
[ATTACHED]
[NAVIGATED] about:blank
[NAVIGATED] http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/phet-io-wrappers/state/?sim=faradays-electromagnetic-lab&locales=*&phetioWrapperDebug=true&fuzz&phetioDebug=true&wrapperContinuousTest=%7B%22test%22%3A%5B%22faradays-electromagnetic-lab%22%2C%22phet-io-state-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1712311463115%22%2C%22timestamp%22%3A1712329940914%7D
[ATTACHED]
[NAVIGATED] about:blank
[ATTACHED]
[NAVIGATED] about:blank
[CONSOLE] enabling assert
[NAVIGATED] http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/faradays-electromagnetic-lab/faradays-electromagnetic-lab_en.html?brand=phet-io&ea&postMessageOnError&sim=faradays-electromagnetic-lab&locales=*&phetioWrapperDebug=true&fuzz&phetioDebug=true&wrapperContinuousTest=%7B%22test%22%3A%5B%22faradays-electromagnetic-lab%22%2C%22phet-io-state-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1712311463115%22%2C%22timestamp%22%3A1712329940914%7D&frameTitle=source
[NAVIGATED] http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/faradays-electromagnetic-lab/faradays-electromagnetic-lab_en.html?brand=phet-io&ea&postMessageOnError&sim=faradays-electromagnetic-lab&locales=*&phetioWrapperDebug=true&fuzz&phetioDebug=true&wrapperContinuousTest=%7B%22test%22%3A%5B%22faradays-electromagnetic-lab%22%2C%22phet-io-state-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1712311463115%22%2C%22timestamp%22%3A1712329940914%7D&frameTitle=destination
[CONSOLE] enabling assert
[CONSOLE] enabling assert
[CONSOLE] continuous-test-wrapper-load
[CONSOLE] continuous-test-wrapper-load
[CONSOLE] Assertion failed: unexpected emfProperty.value: -2094.1448268091262
[CONSOLE] Assertion failed: mismatched index, possible start/end mismatch. Likely this is downstream of another error, try pausing on caught exceptions.
[PAGE ERROR] Error: Error: Assertion failed: mismatched index, possible start/end mismatch. Likely this is downstream of another error, try pausing on caught exceptions.
at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/assert/js/assert.js:28:13)
at DataStream.end (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/phet-io/js/dataStream.js:253:15)
at PhetioCommandProcessor.phetioEndEvent (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/tandem/js/PhetioObject.js:439:30)
at http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/phet-io/js/phetioCommandProcessor.js:135:18
[PAGE ERROR] Error: Error: Assertion failed: unexpected emfProperty.value: -2094.1448268091262
at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/assert/js/assert.js:28:13)
at PickupCoil.clearEMF (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/faradays-electromagnetic-lab/js/common/model/PickupCoil.js:199:15)
at http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/faradays-electromagnetic-lab/js/transformer/model/Transformer.js:57:77
at TinyProperty.notifyLoop (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/axon/js/TinyEmitter.js:176:7)
at TinyProperty.emit (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/axon/js/TinyEmitter.js:154:18)
at Property._notifyListeners (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/axon/js/ReadOnlyProperty.js:258:23)
at PhaseCallback.listener (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/axon/js/ReadOnlyProperty.js:303:47)
at PropertyStateHandler.attemptToApplyPhases (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/axon/js/PropertyStateHandler.js:220:41)
at PropertyStateHandler.undeferAndNotifyProperties (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/axon/js/PropertyStateHandler.js:149:12)
at http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/axon/js/PropertyStateHandler.js:54:12
[PAGE ERROR] Error: Error: Assertion failed: mismatched index, possible start/end mismatch. Likely this is downstream of another error, try pausing on caught exceptions.
at window.assertions.assertFunction (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/assert/js/assert.js:28:13)
at DataStream.end (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/phet-io/js/dataStream.js:253:15)
at PhetioCommandProcessor.phetioEndEvent (http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/tandem/js/PhetioObject.js:439:30)
at http://128.138.93.172/continuous-testing/ct-snapshots/1712311463115/chipper/dist/js/phet-io/js/phetioCommandProcessor.js:135:18
[CONSOLE] continuous-test-wrapper-error
[CONSOLE] continuous-test-wrapper-error
[CONSOLE] continuous-test-wrapper-error
[CONSOLE] continuous-test-wrapper-error

id: "Sparky Node Puppeteer"
Snapshot from 4/5/2024, 4:04:23 AM

@pixelzoom
Copy link
Contributor

pixelzoom commented Apr 5, 2024

Meeting with @zepumph scheduled for Tue 4/9 @ 11AM MT.

pixelzoom added a commit that referenced this issue Apr 9, 2024
@pixelzoom
Copy link
Contributor

pixelzoom commented Apr 9, 2024

The reason that clearEMF fails is that Property changes made while setting state are silently (!) ignored. In ReadOnlyProperty.ts:

  protected set( value: T ): void {

    // state is managed by the PhetioStateEngine.
    // We still want to set Properties when clearing dynamic elements, see https://github.com/phetsims/phet-io/issues/1906
    const setManagedByPhetioState = isPhetioStateEngineManagingPropertyValuesProperty.value &&
                                    !isClearingPhetioDynamicElementsProperty.value &&
                                    this.isPhetioInstrumented() && this.phetioState &&

                                    // However, DerivedProperty should be able to update during PhET-iO state set
                                    this.isSettable();

    if ( !setManagedByPhetioState ) {
      this.unguardedSet( value );
    }
  }

So in 8e7ada8, it's necessary to short-circuit the call to clearEMF when setting state:

    // Ignore the EMF induced by switching power supplies. Do not do this when setting PhET-iO state because Property
    // changes made by clearEMF will be ignored, resulting in failure of clearEMF.
    // See https://github.com/phetsims/faradays-electromagnetic-lab/issues/149.
    this.electromagnet.currentSourceProperty.lazyLink( () => {
      if ( !isSettingPhetioStateProperty.value ) {
        this.pickupCoil.clearEMF();
      }
    } );

I'll leave this open for a few CT cycles to confirm that this resolves the problem.

@pixelzoom
Copy link
Contributor

CT is happy. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants