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

Assertion failed in Spotlight.getIntersection() #156

Closed
zepumph opened this issue Aug 4, 2021 · 9 comments
Closed

Assertion failed in Spotlight.getIntersection() #156

zepumph opened this issue Aug 4, 2021 · 9 comments
Assignees
Labels
dev:code-review type:bug Something isn't working

Comments

@zepumph
Copy link
Member

zepumph commented Aug 4, 2021

I hit this assertion during my first time playing with the sim for #154. I don't feel like I did anything out of the ordinary, or even tried to break it. I pretty much just moved the lens a small amount, and then dragged around the pencil, eventually leading to the bottom left, as shown in the picture. Sorry I didn't have my dev tools open so that I could have paused in the stack trace to give more information. Note the "reentry detected" assertion is likely just downstream of the original problem, and you can probably ignore it.

image

Assertion failed
assert.js:25 Uncaught Error: Assertion failed
    at window.assertions.assertFunction (assert.js:25)
    at Function.intersect (BoundsIntersection.js:213)
    at Function.intersect (Segment.js:739)
    at Graph.eliminateIntersection (Graph.js:694)
    at Graph.computeSimplifiedFaces (Graph.js:189)
    at Function.binaryResult (Graph.js:1286)
    at Spotlight.getIntersection (Spotlight.js:193)
    at Spotlight.js:55
    at getDerivedValue (DerivedProperty.js:30)
    at DerivedProperty.getDerivedPropertyListener (DerivedProperty.js:112)
assert.js:21 Assertion failed: reentry detected, value=Vector2(112.78102820930101, 12.882638828305552), oldValue=Vector2(112.78102820930101, 12.882638828305552)
assert.js:25 Uncaught Error: Assertion failed: reentry detected, value=Vector2(112.78102820930101, 12.882638828305552), oldValue=Vector2(112.78102820930101, 12.882638828305552)
    at window.assertions.assertFunction (assert.js:25)
    at DerivedProperty._notifyListeners (Property.js:268)
    at DerivedProperty.set (Property.js:186)
    at DerivedProperty.getDerivedPropertyListener (DerivedProperty.js:112)
    at TinyProperty.emit (TinyEmitter.js:86)
    at Vector2Property._notifyListeners (Property.js:271)
    at Vector2Property.set (Property.js:186)
    at Vector2Property.set value [as value] (Property.js:341)
    at Optic.setVerticalCoordinate (Optic.js:331)
    at OpticNode.js:119
@zepumph zepumph added type:bug Something isn't working dev:code-review labels Aug 4, 2021
@veillette
Copy link
Contributor

Thanks @zepumph.
This is related to the spotlight and projectorScreen. The spotlight model gets update, independent of the visibility of the projectorScreenNode. We have made progress on a method to reproduce it ( see #72) but we have not manage to track it down.

@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
Copy link
Contributor

This issue is a duplicate of #177.

In #177 (comment), @KatieWoe reported this CT failure:

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

In #177 (comment), @jbphet reported:

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?

@pixelzoom
Copy link
Contributor

pixelzoom commented Sep 29, 2021

This seems related to (duplicate of?) #163. But I'm hesitant to close, because there seems to be more than 1 problem that the sim is hitting in kite when Spotlight.getIntersection() is called.

@arouinfar
Copy link
Contributor

In #215 (comment) we decided not to address assertion failure issues for the prototype, so back to @pixelzoom.

@pixelzoom
Copy link
Contributor

Since this is not in the prototype, unassigning until I start working on public production issues.

@pixelzoom pixelzoom removed their assignment Oct 1, 2021
@pixelzoom
Copy link
Contributor

This is probably the same as #163, since they both mention Spotlight.getIntersection, and both fail in Graph.eliminateIntersection. But the 2 GitHub issues contain different information and reports, so I'll leave them both open. Since #163 is assigned to @jonathanolson, assigning this issue to him as well.

@jonathanolson
Copy link
Contributor

Has this been reproduced since phetsims/kite#90 was fixed?

@pixelzoom
Copy link
Contributor

I've never been able to reproduce this. So I'll go ahead and close this, assuming that it's a duplicate of #163, and QA is testing that issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev:code-review type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants