search

phetsims / charges-and-fields

"Charges And Fields" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
8 stars 7 forks source link

CT Assertion failed #201

Closed KatieWoe closed 1 year ago

KatieWoe commented 1 year ago 
charges-and-fields : phet-io-studio-fuzz : unbuilt
http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/studio/?sim=charges-and-fields&phetioWrapperDebug=true&fuzz&wrapperContinuousTest=%7B%22test%22%3A%5B%22charges-and-fields%22%2C%22phet-io-studio-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1689342541064%22%2C%22timestamp%22%3A1689343051446%7D
Uncaught Error: Assertion failed
Error: Assertion failed
at window.assertions.assertFunction (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/assert/js/assert.js:28:13)
at assert (StateSchema.ts:235:28)
at checkStateObjectValid (IOType.ts:432:42)
at isStateObjectValid (IOType.ts:465:9)
at validateStateObject (IOType.ts:282:23)
at toStateObject (ReadOnlyProperty.ts:305:34)
at getData (PhetioObject.ts:472:45)
at phetioStartEvent (ReadOnlyProperty.ts:300:66)
at _notifyListeners (ReadOnlyProperty.ts:262:13)
at unguardedSet (ReadOnlyProperty.ts:246:11)
[URL] http://128.138.93.172//continuous-testing/aqua/html/wrapper-test.html?url=..%2F..%2Fct-snapshots%2F1689342541064%2Fstudio%2F%3Fsim%3Dcharges-and-fields%26phetioWrapperDebug%3Dtrue%26fuzz&testInfo=%7B%22test%22%3A%5B%22charges-and-fields%22%2C%22phet-io-studio-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1689342541064%22%2C%22timestamp%22%3A1689343051446%7D
[NAVIGATED] http://128.138.93.172//continuous-testing/aqua/html/wrapper-test.html?url=..%2F..%2Fct-snapshots%2F1689342541064%2Fstudio%2F%3Fsim%3Dcharges-and-fields%26phetioWrapperDebug%3Dtrue%26fuzz&testInfo=%7B%22test%22%3A%5B%22charges-and-fields%22%2C%22phet-io-studio-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1689342541064%22%2C%22timestamp%22%3A1689343051446%7D
[NAVIGATED] about:blank
[NAVIGATED] http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/studio/?sim=charges-and-fields&phetioWrapperDebug=true&fuzz&wrapperContinuousTest=%7B%22test%22%3A%5B%22charges-and-fields%22%2C%22phet-io-studio-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1689342541064%22%2C%22timestamp%22%3A1689343051446%7D
[NAVIGATED] about:blank
[CONSOLE] enabling assert
[NAVIGATED] about:blank
[NAVIGATED] http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/charges-and-fields/charges-and-fields_en.html?brand=phet-io&ea&debugger&postMessageOnError&sim=charges-and-fields&phetioWrapperDebug=true&fuzz&wrapperContinuousTest=%7B%22test%22%3A%5B%22charges-and-fields%22%2C%22phet-io-studio-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1689342541064%22%2C%22timestamp%22%3A1689343051446%7D&phetioEmitAPIBaseline&phetioCreateArchetypes&phetioEmitHighFrequencyEvents=false
[CONSOLE] enabling assert
[CONSOLE] continuous-test-wrapper-load
[CONSOLE] Assertion failed
[PAGE ERROR] Error: Error: Assertion failed
at window.assertions.assertFunction (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/assert/js/assert.js:28:13)
at StateSchema.checkStateObjectValid (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/tandem/js/types/StateSchema.js:187:29)
at IOType.isStateObjectValid (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/tandem/js/types/IOType.js:263:43)
at IOType.validateStateObject (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/tandem/js/types/IOType.js:293:10)
at IOType.toStateObject (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/tandem/js/types/IOType.js:132:24)
at Object.getData (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/axon/js/ReadOnlyProperty.js:231:35)
at NumberProperty.phetioStartEvent (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/tandem/js/PhetioObject.js:401:46)
at NumberProperty._notifyListeners (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/axon/js/ReadOnlyProperty.js:226:67)
at NumberProperty.unguardedSet (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/axon/js/ReadOnlyProperty.js:188:14)
at NumberProperty.set (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/axon/js/ReadOnlyProperty.js:173:12)
[PAGE ERROR] Error: Error: Assertion failed
at window.assertions.assertFunction (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/assert/js/assert.js:28:13)
at StateSchema.checkStateObjectValid (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/tandem/js/types/StateSchema.js:187:29)
at IOType.isStateObjectValid (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/tandem/js/types/IOType.js:263:43)
at IOType.validateStateObject (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/tandem/js/types/IOType.js:293:10)
at IOType.toStateObject (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/tandem/js/types/IOType.js:132:24)
at Object.getData (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/axon/js/ReadOnlyProperty.js:231:35)
at NumberProperty.phetioStartEvent (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/tandem/js/PhetioObject.js:401:46)
at NumberProperty._notifyListeners (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/axon/js/ReadOnlyProperty.js:226:67)
at NumberProperty.unguardedSet (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/axon/js/ReadOnlyProperty.js:188:14)
at NumberProperty.set (http://128.138.93.172//continuous-testing/ct-snapshots/1689342541064/chipper/dist/js/axon/js/ReadOnlyProperty.js:173:12)
[CONSOLE] continuous-test-wrapper-error

id: "Sparky Node Puppeteer"
Snapshot from 7/14/2023, 7:49:01 AM
samreid commented 1 year ago

the electricPotentialSensor.electricPotentialProperty is going infinite (as it is supposed to) but the NumberProperty does not support infinity. It's unclear if we can tell the NumberProperty to allow InfiniteNumberIO or if we should use Property.

samreid commented 1 year ago

This patch demonstrates how to reliably trigger the error on startup as well as a NumberProperty hack to solve the problem. I would like to check in with @zepumph about if/how NumberProperty should support infinity.

```diff Subject: [PATCH] Improve assertion messages, see https://github.com/phetsims/charges-and-fields/issues/201 --- Index: axon/js/NumberProperty.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/axon/js/NumberProperty.ts b/axon/js/NumberProperty.ts --- a/axon/js/NumberProperty.ts (revision 2c5d217ca16faaa5be669b3a1b804027ce0bafc6) +++ b/axon/js/NumberProperty.ts (date 1695340770582) @@ -13,7 +13,6 @@ import Tandem from '../../tandem/js/Tandem.js'; import IOType from '../../tandem/js/types/IOType.js'; import NullableIO from '../../tandem/js/types/NullableIO.js'; -import NumberIO from '../../tandem/js/types/NumberIO.js'; import StringIO from '../../tandem/js/types/StringIO.js'; import axon from './axon.js'; import ReadOnlyProperty, { ReadOnlyPropertyState } from './ReadOnlyProperty.js'; @@ -22,6 +21,7 @@ import TRangedProperty from './TRangedProperty.js'; import assertMutuallyExclusiveOptions from '../../phet-core/js/assertMutuallyExclusiveOptions.js'; import { PhetioID } from '../../tandem/js/TandemConstants.js'; +import InfiniteNumberIO from '../../tandem/js/types/InfiniteNumberIO.js'; const VALID_INTEGER = { valueType: 'number', isValidValue: ( v: number ) => v % 1 === 0, validationMessage: 'Should be a valid integer' }; const VALID_NON_NAN = { isValidValue: ( v: number ) => !isNaN( v ), validationMessage: 'Should not be NaN' }; @@ -44,7 +44,7 @@ export type NumberPropertyState = NumberPropertySelfState & ReadOnlyPropertyState; // For the IOType -const PropertyIOImpl = Property.PropertyIO( NumberIO ); +const PropertyIOImpl = Property.PropertyIO( InfiniteNumberIO ); type SelfOptions = { numberType?: NumberType; @@ -54,7 +54,7 @@ rangePropertyOptions?: PropertyOptions; }; -export type NumberPropertyOptions = SelfOptions & StrictOmit, 'phetioValueType' | 'valueType'>; +export type NumberPropertyOptions = SelfOptions & StrictOmit, 'valueType'>; export default class NumberProperty extends Property implements TRangedProperty { @@ -102,7 +102,7 @@ // client cannot specify superclass options that are controlled by NumberProperty options.valueType = 'number'; - options.phetioValueType = NumberIO; // not actually used, but for completeness, don't have ReadOnlyProperty storing the wrong default. + options.phetioValueType = InfiniteNumberIO; // not actually used, but for completeness, don't have ReadOnlyProperty storing the wrong default. const rangePropertyProvided = options.range && options.range instanceof ReadOnlyProperty; const ownsRangeProperty = !rangePropertyProvided; @@ -230,7 +230,7 @@ valueType: NumberProperty, supertype: PropertyIOImpl, - parameterTypes: [ NumberIO ], + parameterTypes: [ InfiniteNumberIO ], documentation: `Extends PropertyIO to add values for the numeric range ( min, max ) and numberType ( '${ VALID_NUMBER_TYPES.join( '\' | \'' )}' )`, toStateObject: numberProperty => { Index: charges-and-fields/js/charges-and-fields/model/ChargesAndFieldsModel.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/charges-and-fields/js/charges-and-fields/model/ChargesAndFieldsModel.js b/charges-and-fields/js/charges-and-fields/model/ChargesAndFieldsModel.js --- a/charges-and-fields/js/charges-and-fields/model/ChargesAndFieldsModel.js (revision 59ec70821ccc065dd85dfb78bcb1d0b237f55833) +++ b/charges-and-fields/js/charges-and-fields/model/ChargesAndFieldsModel.js (date 1695340247938) @@ -526,7 +526,7 @@ const netChargeOnSite = this.getCharge( position ); // the net charge at position - if ( netChargeOnSite > 0 ) { + if ( netChargeOnSite > 0 || true) { return Number.POSITIVE_INFINITY; } else if ( netChargeOnSite < 0 ) { ```
samreid commented 1 year ago

Alternatively we could use Property like this:

    this.electricPotentialProperty = new Property( 0, {
      tandem: tandem.createTandem( 'electricPotentialProperty' ),
      units: 'V',
      phetioReadOnly: true,
      phetioValueType: InfiniteNumberIO
    } );

Let's check in with @zepumph next.

samreid commented 1 year ago

@marlitas @jonathanolson and I decided to go with that last idea. We feel it is good and NumberProperty does not need to have abstraction around infinity. Closing.