dt is sometimes zero

[samreid]

@jbphet noticed in Balancing Act that joist sometimes sends out dt=0 in the step function. It generally appears to be correlated with when the cursor changes to a hand/arrow. I have reproduced the problem on my machine (though it appears less frequently than on @jbphet machine). Perhaps related to phetsims/scenery#275

[jonathanolson]

Does this happen on all browsers?

[samreid]

I noticed this issue in Win8/Chrome Version 38.0.2125.101 m (64-bit)

[samreid]

I updated and also saw this problem once in Chrome Version 38.0.2125.104 m (64-bit)

[pixelzoom]

This sounds like it's either (a) vestigial or (b) potentially serious. Assigning to @jonathanolson.

[jonathanolson]

Will try to reproduce.

[jonathanolson]

I've spent 10 minutes banging on the sim and I'm unable to reproduce (Win7/Chrome).

Added debugger in Sim.js step if dt was zero, I'd recommend @jbphet add the debugger statement and see if you can still trigger it.

[jbphet]

I was able to duplicate this, but not during normal sim usage. I was able to make it happen when I had the developer tools open and was changing the size of the tools window, sliding the divider either right or left. The attached screen shots show the output. I also tried this with a debugger statement, and the statement was indeed hit, demonstrating that this isn't just a rounding issue when printing the value.

The code that was added in the BalanceModel.step function looked like this (including the debug statement):

      if ( dt < 0.0083333 || dt > 0.025 ){
        console.log( 'dt off by more than 50% of nominal, value = ' + dt );
        if ( dt === 0 ){
          debugger;
        }
      }

[jbphet]

Assigning back to @jonathanolson to decide if this is indeed "potentially serious" and should be addressed. The sim handles it just fine, and I assume most sims would, so it may be a non-issue. It's just a little odd that it happens at all.

By the way, the test environment where this was reproduced was Win7+Chrome43.

[jonathanolson]

Will check to make sure resizing doesn't cause sim steps.

[jonathanolson]

Reproduced with Chrome/Mac, had two animation frames with the exact same timestamp (1432926900170) that were naturally generated. Looks like resizing causes more animation frames to be rendered (probably on each resize), so if the sim JS executes quickly enough, there can be many low-DT frames.

Because there is a resize, the ScreenView transform will change, and the view step may be expected to be called after that, so although I'd like to prevent 0-DT frames from even being stepped, I think we need to anyways. Thus in general, sims should be prepared to handle frame DTs that are zero.

[jonathanolson]

Anything else to do here @pixelzoom, @samreid or @jbphet?

[pixelzoom]

Nothing that I know of, but I haven't experienced this or really been paying attention. @samreid created this issue, so perhaps get him or @jbphet to close.

[jbphet]

I'll go ahead and close. I'm fine with the implementation requirement that sims need to be able to handle zero values for dt.

[samreid]

Sure, sims should handle small dt, but shouldn't we just add a line of logic in joist:

if (dt>0){
step();
}

? Then we can avoid any required corner-case handling in sim code?

[pixelzoom]

@jonathanolson said:

sims should be prepared to handle frame DTs that are zero.

Searching for "/ dt", I find 2 sims that will fail with divide-by-zero errors: balloons-and-static-electricity, faradays-law. (Don't know if those sims are testing for "dt > 0" somewhere.)

[pixelzoom]

+1 for @samreid suggestion to address this in joist. Unless someone can explain the value of calling step when dt is zero?

[jonathanolson]

As noted re: skipping step() if dt === 0:

Because there is a resize, the ScreenView transform will change, and the view step may be expected to be called after that, so although I'd like to prevent 0-DT frames from even being stepped, I think we need to anyways.

So we would skip model/view steps, do we also skip updateDisplay() and have the sim NOT update on window resize? Seems sketchy to me.

If we don't skip updateDisplay(), then because the ScreenView transform was changed, other view bits may have changed and would expect model/view updates (view.step() in particular). I assumed every time we redraw the sim, we'll have a view.step().

[pixelzoom]

OK, makes sense. Then we need to take a look at balloons-and-static-electricity and faradays-law (and possibly other sims?)

[jonathanolson]

Yes, probably for most sims. Molecule Shapes might also not handle dt=0 gracefully.

[jonathanolson]

I'd like to update my proposal to consider skipping the model.step() if it's 0, but still doing the view step. Usually view steps don't need the DT as much, and generally models shouldn't change if there's a 0-time DT.

Thoughts?

[samreid]

@jonathanolson's suggestion #171 (comment) seems like a good solution.

[pixelzoom]

Sounds reasonable to me. Regardless, my approach will be to program defensively :)

[jonathanolson]

Committed the proposed change.

Sims that use DT in the view step (ignoring empty implementations):
balancing-chemical-equations (for RewardNode step)
bending-light (timer)
color-vision (beams get stepped)
energy-forms-and-changes (updates appearance)
fluid-pressure-and-flow (steps layers)
gravity-and-orbits (step() called without DT on paths!!?)

I'll check through all usages to ensure that we're fine before closing the issue.

There are also a ton of copy-pasted empty step functions, which aren't needed (if the function doesn't exist, it won't be called). Should we remove those?

[jonathanolson]

All usages look safe, although there are a lot of step functions that are empty or that aren't part of a view and ignore their DT parameter. Will note in meeting agenda about possible cleanup.

[pixelzoom]

Those empty step functions are probably being propagated from simula-rasa. I'll add a comment in simula-rasa that say something like "delete if not used".

[pixelzoom]

"Called by the animation loop. Optional" is the comment in simula-rasa, and it appears 39 times in sim code. So yes, this is where the empty step functions are coming from.

[samreid]

At today's meeting, we decided to remove the empty step functions in existing code, and that @samreid would do it.

[samreid]

OK I removed the empty blocks above. @pixelzoom anything else to do here?

[pixelzoom]

I think the covers it. Closing.