Spotlight.getIntersection() results in assertion failure in kite.

[zepumph]

While playing with the sim with the second source turned on, I found that the sim just froze, with no errors. I was able to pause in the chrome dev tools fast enough that the page didn't crash. It looks like there is a loop in BoundsIntersections.pushSubdivisions(). Note the current length of the "intersections" array below (13962399).

I will try to data dump as much as possible here from the stack for reproduceability. I think the important bits are coming from Spotlight.getIntersection(), so I'll go from there.

screenPosition
>Vector2 {x: 200, y: 0}

opticPosition
>Vector2 {x: 0, y: 0}

opticDiameter
>80

targetPosition
>Vector2 {x: 187.97374935890778, y: -66.22766706581093}

screenShape.toString()
>"new kite.Shape( 'M 172.00000000000000000000 39.00000000000000000000 L 172.00000000000000000000 -51.00000000000000000000 L 225.00000000000000000000 -83.00000000000000000000 L 225.00000000000000000000 72.00000000000000000000 L 172.00000000000000000000 39.00000000000000000000 Z ' )"

diskShape.toString()
>"new kite.Shape( 'M 201.27956700139799295357 -70.46480403958862837044 A 2.55913400279601432885 1.27956700139800716443 450 0 1 198.72043299860195020301 -70.46480403958862837044 A 2.55913400279601432885 1.27956700139800716443 450 0 1 201.27956700139799295357 -70.46480403958862837044 Z ' )"

I hope this is helpful, and good luck.

[zepumph]

#154

[veillette]

I rendered the shapes.
Visually, we get (I cropped the image):

[veillette]

That was really useful @zepumph . I see that the two shapes are well defined, but the shapes are barely kissing. Since we want the intersections of the two shapes, this suggest that kissing shapes are problematic with intersections.

[pixelzoom]

I was asked to evaluate this for https://github.com/phetsims/geometric-optics/milestone/1, so self-assigning.

[pixelzoom]

@zepumph said:

While playing with the sim with the second source turned on, I found that the sim just froze, with no errors. ...

It would be really helpful to have steps to reproduce. I've been playing with the sim for 15 minutes and can't reproduce this problem. @zepumph do you recall how you reproduced this?

[zepumph]

@zepumph do you recall how you reproduced this?

I do not! And I remember feeling like it would be really hard to reproduce during it, which is why I tried to gather so much info from the environment. I think that @veillette had a good idea about how to draw the two shapes I posted in such a way that they created the single point of overlap. I'm sorry I can't be of more help here.

[pixelzoom]

While I can't reproduce this, the stack trace points to this being a problem in kite (the common-code repo for shapes). Here's the part of the Stack trace (from the 1st comment above) that's relevant:

Spotlight.js is sim-specific code, and here's where it passes things off to kite:

      // find the intersection of the two shapes
      return Graph.binaryResult( screenShape, diskShape, Graph.BINARY_NONZERO_INTERSECTION );

I took a quick dive into the kite methods, but this is code that I'm not familiar with. You'll need to have @jonathanolson take it from here. Assigning to @kathy-phet and @arouinfar to prioritize.

[pixelzoom]

9/23/21 design meeting:

@KatieWoe please test on master. If you can reproduced, please adds steps here.

[Nancy-Salpepi]

I was able to get the sim to freeze twice just by moving the pencil around with the second source placed in the center of the pencil. It is not necessary to change any other settings in the sim. The sim froze when the pencil was in different places, so I have not been able to nail down a set of steps to reproduce. However, it wasn't a runtime error that I saw. It was an assertion error.

[pixelzoom]

@Nancy-Salpepi can you please provide the usual Troubleshooting info? And how were you running the sim? If assertions were enabled, I'm guessing that you were using phettest or a local copy.

[pixelzoom]

Since this is a assertion failure, it's being run in an unbuilt version, because assertions are stripped out in built version. We may want to see what the repercussions are in a built version. What does the user see (if anything) when this occurs, and does the sim continue to run OK? So...

@Nancy-Salpepi could you please test build version 1.0.0-dev12? What happens where you follow the same testing procedure that you described in #163 (comment)? Be sure to have the browser console open.

[Nancy-Salpepi]

Test device
MacBook Air (m1 chip)

Operating System
11.5.2

Browser
chrome

Problem description
On Master:
I was able to get the sim to freeze four times just by moving the pencil around with the second source placed in the center of the pencil. It is not necessary to change any other settings in the sim. The sim froze when the pencil was in different places, so I have not been able to nail down a set of steps to reproduce. However, it wasn't a runtime error that I saw. It was an assertion error.

See above for console error #163 (comment)

Steps to reproduce
It has frozen in 4 different places so far. Still working on reproducible steps.

Visuals
see above #163 (comment)

Troubleshooting information:
!!!!! DO NOT EDIT !!!!!
Name: Geometric Optics
URL: https://bayes.colorado.edu/dev/phettest/geometric-optics/geometric-optics_en.html?ea&brand=phet
Version: 1.0.0-dev.12 (unbuilt)
Features missing: applicationcache, applicationcache, touch
Flags: pixelRatioScaling
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/93.0.4577.82 Safari/537.36
Language: en-US
Window: 1440x690
Pixel Ratio: 2/1
WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium)
GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium)
Vendor: WebKit (WebKit WebGL)
Vertex: attribs: 16 varying: 31 uniform: 1024
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 80)
Max viewport: 16384x16384
OES_texture_float: true
Dependencies JSON: {}

[Nancy-Salpepi]

With MacBook + chrome:
Using 1.0.0-dev.12 and the same testing procedure described in #163 (comment), I was unable to reproduce the sim freezing.

I will continue to use this built version and make additional manipulations to try and reproduce the original error described in #163 (comment).

[kathy-phet]

@Nancy-Salpepi - A while back I was able to freeze the sim by changing the refraction index in different circumstances. I know code has changed since then, but you might try wiggling other knobs too to see if you can induce any crashes at all in the built version. Thanks for banging on this issue!

[KatieWoe]

I wanted to take a look at this myself and went to master to get a look at the bug. I wasn't able to reproduce it there, and @Nancy-Salpepi said she hasn't been able to either. Has something changed?

[pixelzoom]

@KatieWoe I beleive that @Nancy-Salpepi said that she DID reproduce the problem in master (unbuilt, assertions enabled), but DID NOT reproduce the problem in a dev version (built, assertions stripped). @Nancy-Salpepi do I understand that correctly?

And no, nothing has been changed.

[KatieWoe]

She said this to me about half an hour ago. That she could reproduce it this morning, but was unable to now. @Nancy-Salpepi can you confirm my understanding?

[jonathanolson]

I briefly looked at the case @zepumph noted, and created phetsims/kite#90. It's not handling well intersections between two segments that are essentially a single elliptical arc split incredibly close to a horizontal tangent. We could both (a) improve arbitrary intersection, but to handle this case better (b) handling ellipse/circle intersections where they only differ by start/end angles.

[pixelzoom]

I'm still not clear on whether this needs to be addressed for the prototype. Labeling for design meeting.

[pixelzoom]

There's a lot of overlap here with #156, and they seem closely related. For example, the error that @Nancy-Salpepi reported in #163 (comment) is actually the topic of #156. And @KatieWoe encountered 2 different errors in #163 (comment) and #163 (comment).

So I'm not sure how to proceed with this issue and #163. They sim seems to be experiencing several different failures in kite. And since we can't come up with steps to reproduce, it's difficult to tell if they are related.

[jonathanolson]

Additionally, let me know if I should bump up kite failures in priority here.

[pixelzoom]

9/30/21 design meeting:

Since this doesn't cause a hard failure in practice (assertions are stripped in built versions), we will not address this for the Prototype milestone. It should be addressed for the first public publication.

[jonathanolson]

I've handled what was causing the particular failure identified by @zepumph in this issue. I'm going to leave self-assigned until I see if there might be other triggers (and review the other related issues).

[pixelzoom]

I've handled what was causing the particular failure identified by @zepumph in this issue.

Where did you handle it? Is there a sha that you can reference? There are no commits linked to this issue.

I'm going to leave self-assigned until I see if there might be other triggers (and review the other related issues).

What's the timeframe for completing this?

[jonathanolson]

Where did you handle it? Is there a sha that you can reference? There are no commits linked to this issue.

Commits were tagged to the common issue linked to this: phetsims/kite#90

What's the timeframe for completing this?

Presumably if it's not failing, we could close this?

[pixelzoom]

I was never able to successfully reproduce this, but QA did. @KatieWoe could you (or someone you assign) please verify that this has been fixed?

[KatieWoe]

A few of us have tested this since this morning and so far we have not reproduced the issue. It looks fixed so far, but since it was difficult to reproduce in the first place, it is hard to be sure.

[pixelzoom]

Thanks QA team. Let's consider this resolved. Closing.