search

phetsims / density

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

CT Designed API changes detected, please roll them back or revise the reference API: #133

Closed KatieWoe closed 1 year ago

KatieWoe commented 2 years ago 
density : phet-io-api-compatibility : unbuilt
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1649860937297/density/density_en.html?continuousTest=%7B%22test%22%3A%5B%22density%22%2C%22phet-io-api-compatibility%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1649860937297%22%2C%22timestamp%22%3A1649865165037%7D&ea&brand=phet-io&phetioStandalone&phetioCompareAPI&randomSeed=332211
Query: ea&brand=phet-io&phetioStandalone&phetioCompareAPI&randomSeed=332211
Uncaught Error: Assertion failed: Designed API changes detected, please roll them back or revise the reference API:

density.compareScreen.model.invisibleBarrierBoundsProperty._data.initialState differs.
Expected:
{"units":null,"validValues":null,"value":{"maxX":0.8660253438317398,"maxY":4,"maxZ":0.2,"minX":-0.8660253438317398,"minY":-4,"minZ":-0.2}}
actual:
{"value":{"minX":-0.8616777966790742,"minY":-4,"minZ":-0.2,"maxX":0.8616777966790742,"maxY":4,"maxZ":0.2},"validValues":null,"units":null}

density.introScreen.model.invisibleBarrierBoundsProperty._data.initialState differs.
Expected:
{"units":null,"validValues":null,"value":{"maxX":0.4936825989440907,"maxY":4,"maxZ":0.2,"minX":-0.8660253438317398,"minY":-4,"minZ":-0.2}}
actual:
{"value":{"minX":-0.8616777966790742,"minY":-4,"minZ":-0.2,"maxX":0.4911829180109931,"maxY":4,"maxZ":0.2},"validValues":null,"units":null}

density.mysteryScreen.model.invisibleBarrierBoundsProperty._data.initialState differs.
Expected:
{"units":null,"validValues":null,"value":{"maxX":0.8660253438317398,"maxY":4,"maxZ":0.2,"minX":-0.8660253438317398,"minY":-4,"minZ":-0.2}}
actual:
{"value":{"minX":-0.8616777966790742,"minY":-4,"minZ":-0.2,"maxX":0.8616777966790742,"maxY":4,"maxZ":0.2},"validValues":null,"units":null}

density.mysteryScreen.model.scale._data.initialState differs.
Expected:
{"canMove":false,"canRotate":false,"force":{"x":0,"y":0},"matrix":{"entries":[1,0,0,0,1,0,-0.7410253438317398,0.03,1],"type":"TRANSLATION_2D"},"originalMatrix":{"entries":[1,0,0,0,1,0,-0.7410253438317398,0.03,1],"type":"TRANSLATION_2D"},"position":{"x":-0.7410253524780274,"y":0.030000001192092896},"stepMatrix":{"entries":[1,0,0,0,1,0,0,0,1],"type":"IDENTITY"},"tag":"NONE","velocity":{"x":0,"y":0}}
actual:
{"matrix":{"entries":[1,0,0,0,1,0,-0.7366777966790741,0.03,1],"type":"TRANSLATION_2D"},"stepMatrix":{"entries":[1,0,0,0,1,0,0,0,1],"type":"IDENTITY"},"originalMatrix":{"entries":[1,0,0,0,1,0,-0.7366777966790741,0.03,1],"type":"TRANSLATION_2D"},"canRotate":false,"canMove":false,"tag":"NONE","position":{"x":-0.7366777896881104,"y":0.030000001192092896},"velocity":{"x":0,"y":0},"force":{"x":0,"y":0}}

Error: Assertion failed: Designed API changes detected, please roll them back or revise the reference API:
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1649860937297/assert/js/assert.js:25:13)
at assert (phetioEngine.js:333:22)
id: Bayes Chrome
Snapshot from 4/13/2022, 8:42:17 AM
jonathanolson commented 2 years ago

I believe the API is now showing as compatible. I verified all the changes, and I think this error was transitory

KatieWoe commented 2 years ago

Still seeing this I believe:

density : phet-io-api-compatibility : unbuilt
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1658156886162/density/density_en.html?continuousTest=%7B%22test%22%3A%5B%22density%22%2C%22phet-io-api-compatibility%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1658156886162%22%2C%22timestamp%22%3A1658162148577%7D&ea&brand=phet-io&phetioStandalone&phetioCompareAPI&randomSeed=332211
Query: ea&brand=phet-io&phetioStandalone&phetioCompareAPI&randomSeed=332211
Uncaught Error: Assertion failed: Designed API changes detected, please roll them back or revise the reference API:

density.compareScreen.model.invisibleBarrierBoundsProperty._data.initialState differs.
Expected:
{"units":null,"validValues":null,"value":{"maxX":0.8660253438317398,"maxY":4,"maxZ":0.2,"minX":-0.8660253438317398,"minY":-4,"minZ":-0.2}}
actual:
{"value":{"minX":-0.86602534383174,"minY":-4,"minZ":-0.2,"maxX":0.86602534383174,"maxY":4,"maxZ":0.2},"validValues":null,"units":null}

density.introScreen.model.invisibleBarrierBoundsProperty._data.initialState differs.
Expected:
{"units":null,"validValues":null,"value":{"maxX":0.4936825989440907,"maxY":4,"maxZ":0.2,"minX":-0.8660253438317398,"minY":-4,"minZ":-0.2}}
actual:
{"value":{"minX":-0.86602534383174,"minY":-4,"minZ":-0.2,"maxX":0.49368259894409083,"maxY":4,"maxZ":0.2},"validValues":null,"units":null}

density.mysteryScreen.model.invisibleBarrierBoundsProperty._data.initialState differs.
Expected:
{"units":null,"validValues":null,"value":{"maxX":0.8660253438317398,"maxY":4,"maxZ":0.2,"minX":-0.8660253438317398,"minY":-4,"minZ":-0.2}}
actual:
{"value":{"minX":-0.86602534383174,"minY":-4,"minZ":-0.2,"maxX":0.86602534383174,"maxY":4,"maxZ":0.2},"validValues":null,"units":null}

density.mysteryScreen.model.scale._data.initialState differs.
Expected:
{"canMove":false,"canRotate":false,"force":{"x":0,"y":0},"massShape":"BLOCK","matrix":{"entries":[1,0,0,0,1,0,-0.7410253438317398,0.03,1],"type":"TRANSLATION_2D"},"originalMatrix":{"entries":[1,0,0,0,1,0,-0.7410253438317398,0.03,1],"type":"TRANSLATION_2D"},"position":{"x":-0.7410253524780274,"y":0.030000001192092896},"stepMatrix":{"entries":[1,0,0,0,1,0,0,0,1],"type":"IDENTITY"},"tag":"NONE","velocity":{"x":0,"y":0}}
actual:
{"matrix":{"entries":[1,0,0,0,1,0,-0.74102534383174,0.03,1],"type":"TRANSLATION_2D"},"stepMatrix":{"entries":[1,0,0,0,1,0,0,0,1],"type":"IDENTITY"},"originalMatrix":{"entries":[1,0,0,0,1,0,-0.74102534383174,0.03,1],"type":"TRANSLATION_2D"},"canRotate":false,"canMove":false,"tag":"NONE","massShape":"BLOCK","position":{"x":-0.7410253524780274,"y":0.030000001192092896},"velocity":{"x":0,"y":0},"force":{"x":0,"y":0}}

Error: Assertion failed: Designed API changes detected, please roll them back or revise the reference API:
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1658156886162/assert/js/assert.js:28:13)
at assert (phetioEngine.js:333:22)
id: Bayes Puppeteer
Snapshot from 7/18/2022, 9:08:06 AM

----------------------------------

density : phet-io-wrappers-tests : assert
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1658156886162/phet-io-wrappers/phet-io-wrappers-tests.html?sim=density&phetioDebug=true&phetioWrapperDebug=true
8 out of 9 tests passed. 1 failed.
SimTests: density: iframe API failed:
we did not receive a callback from testOnERrorListener
at https://bayes.colorado.edu/continuous-testing/ct-snapshots/1658156886162/chipper/dist/js/phet-io-wrappers/js/tests/SimTests.js:275:16

id: Bayes Puppeteer
Snapshot from 7/18/2022, 9:08:06 AM
jonathanolson commented 2 years ago

@zepumph this isn't coming up in a generate-phet-io-api, how would I fix this?

jonathanolson commented 2 years ago

Actually, hopefully the above commit fixes this?

zepumph commented 2 years ago

From https://github.com/phetsims/density/issues/133#issuecomment-1188227655

Still seeing this I believe:

The error itself is the same, but for different changes. @jonathanolson the Density API is just something to consider as you develop buoyancy. This will continue to happen, and if we aren't careful, and leak into other API updates when we change common code.

jonathanolson commented 2 years ago

This is still erroring, and it's just a permutation of keys instead of different state, e.g.:

{"units":null,"validValues":null,"value":{"maxX":0.8660253438317398,"maxY":4,"maxZ":0.2,"minX":-0.8660253438317398,"minY":-4,"minZ":-0.2}}

vs

{"value":{"minX":-0.8660253438317399,"minY":-4,"minZ":-0.2,"maxX":0.8660253438317399,"maxY":4,"maxZ":0.2},"validValues":null,"units":null}

@zepumph how would I handle this?

zepumph commented 2 years ago

I really don't know! It seems machine dependent. I have no idea how to handle it. Perhaps we could add a guard when generating the API to round to 10 decimal places?

@samreid do you have any ideas about this?

samreid commented 2 years ago

and it's just a permutation of keys instead of different state, e.g.:

It does have different state, 

"maxX":0.8660253438317398,
"maxX":0.8660253438317399

I'm not sure where these values are coming from.

zepumph commented 2 years ago

Could it be another version of puppeteer? My guess is that any number of things in a computer's runtime can alter that decimal place. Is that true?

samreid commented 2 years ago

This code seems susceptible to sequencing problems, and also has the value expanding out based on the number of times called:

    model.invisibleBarrierBoundsProperty.value = model.invisibleBarrierBoundsProperty.value.withMinX( leftPoint.x + 0.01 ).withMaxX( rightPoint.x - 0.01 );
    const resizeBarrier = () => {
      const stage = this.sceneNode.stage;
      if ( stage.canvasWidth && stage.canvasHeight ) {
        const leftRay = this.sceneNode.getRayFromScreenPoint( this.localToGlobalPoint( this.leftBarrierViewPointProperty.value.value ) );
        const rightRay = this.sceneNode.getRayFromScreenPoint( this.localToGlobalPoint( this.rightBarrierViewPointProperty.value.value ) );
        const leftPoint = new Plane3( Vector3.Z_UNIT, 0.09 ).intersectWithRay( leftRay );
        const rightPoint = new Plane3( Vector3.Z_UNIT, 0.09 ).intersectWithRay( rightRay );
        model.invisibleBarrierBoundsProperty.value = model.invisibleBarrierBoundsProperty.value.withMinX( leftPoint.x + 0.01 ).withMaxX( rightPoint.x - 0.01 );
      }
    };

    // leftBarrierViewPointProperty and rightBarrierViewPointProperty are Property<Property>, and we need to listen
    // to when the value.value changes
    // This instance lives for the lifetime of the simulation, so we don't need to remove these listeners
    new DynamicProperty( this.leftBarrierViewPointProperty ).lazyLink( resizeBarrier );
    new DynamicProperty( this.rightBarrierViewPointProperty ).lazyLink( resizeBarrier );
    this.postLayoutEmitter.addListener( resizeBarrier ); // We need to wait for the layout AND render

Can that logic be simplified? Perhaps additionally rounding off invisibleBarrierBoundsProperty itself?

KatieWoe commented 2 years ago 
density : phet-io-api-compatibility : unbuilt
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1663823613768/density/density_en.html?continuousTest=%7B%22test%22%3A%5B%22density%22%2C%22phet-io-api-compatibility%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1663823613768%22%2C%22timestamp%22%3A1663830471169%7D&ea&brand=phet-io&phetioStandalone&phetioCompareAPI&randomSeed=332211
Query: ea&brand=phet-io&phetioStandalone&phetioCompareAPI&randomSeed=332211
Uncaught Error: Assertion failed: Designed API changes detected, please roll them back or revise the reference API:

density.compareScreen.model.invisibleBarrierBoundsProperty._data.initialState differs.
Expected:
{"units":null,"validValues":null,"value":{"maxX":0.8660253438317398,"maxY":4,"maxZ":0.2,"minX":-0.8660253438317398,"minY":-4,"minZ":-0.2}}
actual:
{"value":{"minX":-0.86602534383174,"minY":-4,"minZ":-0.2,"maxX":0.86602534383174,"maxY":4,"maxZ":0.2},"validValues":null,"units":null}

density.introScreen.model.invisibleBarrierBoundsProperty._data.initialState differs.
Expected:
{"units":null,"validValues":null,"value":{"maxX":0.47648041051542245,"maxY":4,"maxZ":0.2,"minX":-0.8660253438317398,"minY":-4,"minZ":-0.2}}
actual:
{"value":{"minX":-0.86602534383174,"minY":-4,"minZ":-0.2,"maxX":0.47648041051542256,"maxY":4,"maxZ":0.2},"validValues":null,"units":null}

density.mysteryScreen.model.invisibleBarrierBoundsProperty._data.initialState differs.
Expected:
{"units":null,"validValues":null,"value":{"maxX":0.8660253438317398,"maxY":4,"maxZ":0.2,"minX":-0.8660253438317398,"minY":-4,"minZ":-0.2}}
actual:
{"value":{"minX":-0.86602534383174,"minY":-4,"minZ":-0.2,"maxX":0.86602534383174,"maxY":4,"maxZ":0.2},"validValues":null,"units":null}

density.mysteryScreen.model.scale._data.initialState differs.
Expected:
{"canMove":false,"canRotate":false,"force":{"x":0,"y":0},"massShape":"BLOCK","matrix":{"entries":[1,0,0,0,1,0,-0.7410253438317398,0.03,1],"type":"TRANSLATION_2D"},"originalMatrix":{"entries":[1,0,0,0,1,0,-0.7410253438317398,0.03,1],"type":"TRANSLATION_2D"},"position":{"x":-0.7410253524780274,"y":0.030000001192092896},"stepMatrix":{"entries":[1,0,0,0,1,0,0,0,1],"type":"IDENTITY"},"tag":"NONE","velocity":{"x":0,"y":0}}
actual:
{"matrix":{"entries":[1,0,0,0,1,0,-0.74102534383174,0.03,1],"type":"TRANSLATION_2D"},"stepMatrix":{"entries":[1,0,0,0,1,0,0,0,1],"type":"IDENTITY"},"originalMatrix":{"entries":[1,0,0,0,1,0,-0.74102534383174,0.03,1],"type":"TRANSLATION_2D"},"canRotate":false,"canMove":false,"tag":"NONE","massShape":"BLOCK","position":{"x":-0.7410253524780274,"y":0.030000001192092896},"velocity":{"x":0,"y":0},"force":{"x":0,"y":0}}

Error: Assertion failed: Designed API changes detected, please roll them back or revise the reference API:
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1663823613768/assert/js/assert.js:28:13)
at assert (phetioEngine.ts:365:22)
id: Bayes Puppeteer
Snapshot from 9/21/2022, 11:13:33 PM

----------------------------------

density : phet-io-api-compatibility : unbuilt
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1663823613768/density/density_en.html?continuousTest=%7B%22test%22%3A%5B%22density%22%2C%22phet-io-api-compatibility%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1663823613768%22%2C%22timestamp%22%3A1663836710372%7D&ea&brand=phet-io&phetioStandalone&phetioCompareAPI&randomSeed=332211
Query: ea&brand=phet-io&phetioStandalone&phetioCompareAPI&randomSeed=332211
Uncaught Error: Assertion failed: Designed API changes detected, please roll them back or revise the reference API:

density.compareScreen.model.invisibleBarrierBoundsProperty._data.initialState differs.
Expected:
{"units":null,"validValues":null,"value":{"maxX":0.8660253438317398,"maxY":4,"maxZ":0.2,"minX":-0.8660253438317398,"minY":-4,"minZ":-0.2}}
actual:
{"value":{"minX":-0.86602534383174,"minY":-4,"minZ":-0.2,"maxX":0.86602534383174,"maxY":4,"maxZ":0.2},"validValues":null,"units":null}

density.introScreen.model.invisibleBarrierBoundsProperty._data.initialState differs.
Expected:
{"units":null,"validValues":null,"value":{"maxX":0.47648041051542245,"maxY":4,"maxZ":0.2,"minX":-0.8660253438317398,"minY":-4,"minZ":-0.2}}
actual:
{"value":{"minX":-0.86602534383174,"minY":-4,"minZ":-0.2,"maxX":0.47648041051542256,"maxY":4,"maxZ":0.2},"validValues":null,"units":null}

density.mysteryScreen.model.invisibleBarrierBoundsProperty._data.initialState differs.
Expected:
{"units":null,"validValues":null,"value":{"maxX":0.8660253438317398,"maxY":4,"maxZ":0.2,"minX":-0.8660253438317398,"minY":-4,"minZ":-0.2}}
actual:
{"value":{"minX":-0.86602534383174,"minY":-4,"minZ":-0.2,"maxX":0.86602534383174,"maxY":4,"maxZ":0.2},"validValues":null,"units":null}

density.mysteryScreen.model.scale._data.initialState differs.
Expected:
{"canMove":false,"canRotate":false,"force":{"x":0,"y":0},"massShape":"BLOCK","matrix":{"entries":[1,0,0,0,1,0,-0.7410253438317398,0.03,1],"type":"TRANSLATION_2D"},"originalMatrix":{"entries":[1,0,0,0,1,0,-0.7410253438317398,0.03,1],"type":"TRANSLATION_2D"},"position":{"x":-0.7410253524780274,"y":0.030000001192092896},"stepMatrix":{"entries":[1,0,0,0,1,0,0,0,1],"type":"IDENTITY"},"tag":"NONE","velocity":{"x":0,"y":0}}
actual:
{"matrix":{"entries":[1,0,0,0,1,0,-0.74102534383174,0.03,1],"type":"TRANSLATION_2D"},"stepMatrix":{"entries":[1,0,0,0,1,0,0,0,1],"type":"IDENTITY"},"originalMatrix":{"entries":[1,0,0,0,1,0,-0.74102534383174,0.03,1],"type":"TRANSLATION_2D"},"canRotate":false,"canMove":false,"tag":"NONE","massShape":"BLOCK","position":{"x":-0.7410253524780274,"y":0.030000001192092896},"velocity":{"x":0,"y":0},"force":{"x":0,"y":0}}

Error: Assertion failed: Designed API changes detected, please roll them back or revise the reference API:
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1663823613768/assert/js/assert.js:28:13)
at assert (phetioEngine.ts:365:22)
id: Bayes Puppeteer
Snapshot from 9/21/2022, 11:13:33 PM
jonathanolson commented 1 year ago

Hmm, it also looks like "of course" the invisibleBarrier has different initial state... different aspect ratios or screen sizes will have different initial states.

@zepumph or @samreid thoughts?

samreid commented 1 year ago

Can we reconsider the invisibleBarrierBoundsProperty to be an implementation detail, and remove the phet-io instrumentation from it?

jonathanolson commented 1 year ago

I think that would be fine, actually! It's already marked as read-only.

jonathanolson commented 1 year ago

Uninstrumented, initial studio testing looks good. Above API update is the documentation rename + uninstrumentation of that Property.

@samreid any other tests to do to see if this causes any other issues? Seems resolved?

samreid commented 1 year ago

The change set looks great, I think this issue can be closed.