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

CT: Assertion failure in kite.BoundsIntersection.intersect #177

Closed
KatieWoe opened this issue Aug 23, 2021 · 3 comments
Closed

CT: Assertion failure in kite.BoundsIntersection.intersect #177

KatieWoe opened this issue Aug 23, 2021 · 3 comments
Assignees
Labels
type:automated-testing type:duplicate This issue or pull request already exists

Comments

@KatieWoe
Copy link
Contributor

geometric-optics : fuzz : unbuilt
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1629711091174/geometric-optics/geometric-optics_en.html?continuousTest=%7B%22test%22%3A%5B%22geometric-optics%22%2C%22fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1629711091174%22%2C%22timestamp%22%3A1629719544789%7D&brand=phet&ea&fuzz&memoryLimit=1000
Query: brand=phet&ea&fuzz&memoryLimit=1000
Uncaught Error: Assertion failed
Error: Assertion failed
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1629711091174/assert/js/assert.js:25:13)
at Function.intersect (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1629711091174/kite/js/ops/BoundsIntersection.js:213:17)
at Function.intersect (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1629711091174/kite/js/segments/Segment.js:739:33)
at Graph.eliminateIntersection (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1629711091174/kite/js/ops/Graph.js:694:41)
at Graph.computeSimplifiedFaces (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1629711091174/kite/js/ops/Graph.js:189:10)
at Function.binaryResult (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1629711091174/kite/js/ops/Graph.js:1286:11)
at Spotlight.getIntersection (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1629711091174/geometric-optics/js/lens/model/Spotlight.js:194:20)
at https://bayes.colorado.edu/continuous-testing/ct-snapshots/1629711091174/geometric-optics/js/lens/model/Spotlight.js:56:21
at getDerivedValue (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1629711091174/axon/js/DerivedProperty.js:30:10)
at DerivedProperty.getDerivedPropertyListener (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1629711091174/axon/js/DerivedProperty.js:112:18)
id: Bayes Chrome
Snapshot from 8/23/2021, 3:31:31 AM
@jbphet
Copy link
Contributor

jbphet commented Aug 23, 2021

I played around with this for a bit and was able to get it to happen manually a few times, but it's not easy. Below is a screenshot of one of these trials. To duplicate, set the sim's controls up as shown and move the pencil around a bunch. It took me about five or ten minutes and a lot of movement, and I didn't notice any particular pattern.

image

When I looked at the stack trace, I noticed that there is a call to Graph.binaryResult, which is static, so the problem must not be due to any previous state. Since it is hard to duplicate, it must require very particular values, so I took some of the values from one of the instances where the problem occurred and modified the code to use those values whenever the pencil image got close to the location where the problem originally occurred. The modifications were done in the constructor of the Spotlight class and looked like this:

    this.shapeProperty = new DerivedProperty( [
        screenPositionProperty,
        optic.positionProperty,
        optic.diameterProperty,
        targetPositionProperty ],
      ( screenPosition, opticPosition, opticDiameter, targetPosition ) => {
        if ( screenPosition.x === 200 && screenPosition.y === 0 &&
             opticPosition.x === 0 && opticPosition.y === 0 &&
             targetPosition.x < -60 && targetPosition.x > -80 &&
             targetPosition.y < 120 & targetPosition.y > 100
        ) {
          targetPosition.x = -68.18783218424616;
          targetPosition.y = 110.31233303003494;
        }
        return this.getIntersection( screenPosition, opticPosition, opticDiameter, targetPosition );
      } );

With this code in place, if the user moves the pencil to the position shown in the image above, the problem will occur every time.

@jonathanolson - The point where the assertion is thrown is several layers deep in some of the kite repo code, and is unfamiliar territory to me. The line of code is:

  assert && assert( positionA.distance( positionB ) < 1e-10 );

I dumped the value of positionA.distance( positionB ) for the experiment described above, and it was 1.460875864722766e-10, so it's very close to the threshold but a little above. Was the threshold arbitrarily chosen, and might it need to be adjusted based on what we're running into here? Any other ideas on how to fix this?

@jbphet jbphet assigned jonathanolson and unassigned jbphet Aug 23, 2021
@pixelzoom pixelzoom changed the title CT Assertion failed CT Assertion failed in BoundsIntersection.intersect Sep 15, 2021
@pixelzoom pixelzoom changed the title CT Assertion failed in BoundsIntersection.intersect CT Assertion failure in BoundsIntersection.intersect Sep 15, 2021
@pixelzoom pixelzoom changed the title CT Assertion failure in BoundsIntersection.intersect CT: Assertion failure in BoundsIntersection.intersect Sep 15, 2021
@pixelzoom
Copy link
Contributor

@arouinfar should this be addressed for the prototype? I have not investigated whether it causes the sim to crash in practice.

@pixelzoom pixelzoom changed the title CT: Assertion failure in BoundsIntersection.intersect CT: Assertion failure in kite.BoundsIntersection.intersect Sep 29, 2021
@phetsims phetsims deleted a comment from KatieWoe Sep 29, 2021
@phetsims phetsims deleted a comment from veillette Sep 29, 2021
@pixelzoom
Copy link
Contributor

This is a duplicate of #156. That issue contains more info, so I'll close this one.

@pixelzoom pixelzoom added type:duplicate This issue or pull request already exists and removed meeting:design labels Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:automated-testing type:duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

5 participants