CT invalid dt:0

[KatieWoe]

diffusion : fuzz : require.js-canvas : run
Query: brand=phet&ea&fuzz&rootRenderer=canvas
Uncaught Error: Assertion failed: invalid dt: 0
Error: Assertion failed: invalid dt: 0
    at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/snapshot-1561528310946/assert/js/assert.js:22:13)
    at DiffusionScreenView.step (https://bayes.colorado.edu/continuous-testing/snapshot-1561528310946/gas-properties/js/common/view/BaseScreenView.js?bust=1561551561107:97:17)
    at https://bayes.colorado.edu/continuous-testing/snapshot-1561528310946/joist/js/Sim.js?bust=1561551561107:232:21
    at Action.execute (https://bayes.colorado.edu/continuous-testing/snapshot-1561528310946/axon/js/Action.js?bust=1561551561107:177:20)
    at Sim.stepSimulation (https://bayes.colorado.edu/continuous-testing/snapshot-1561528310946/joist/js/Sim.js?bust=1561551561107:934:33)
    at Sim.stepOneFrame (https://bayes.colorado.edu/continuous-testing/snapshot-1561528310946/joist/js/Sim.js?bust=1561551561107:915:14)
    at Sim.runAnimationLoop (https://bayes.colorado.edu/continuous-testing/snapshot-1561528310946/joist/js/Sim.js?bust=1561551561107:898:14)
id: Bayes Chrome
Approximately 6/25/2019, 11:51:50 PM

[pixelzoom]

Hmm... This one is very odd. Sim is providing a value of 0 for dt. Here's the assertion that's failing, in BaseScreenView:

    step( dt ) {
      assert && assert( typeof dt === 'number' && dt > 0, `invalid dt: ${dt}` );

I was surprised to see this in Sim stepOnFrame:

      // Don't run the simulation on steps back in time (see https://github.com/phetsims/joist/issues/409)
      if ( dt >= 0 ) {
        this.stepSimulation( dt );
      }

So either the expression ( dt >= 0 ) is incorrect, or 0 is a valid value for dt that all sims need to handle. If it's the latter, I wasn't aware of that, and dt=0 would be a common cause of divide-by-zero errors. What is the purpose of calling stepSimulation( 0 )? And under what conditions would dt be 0?

@samreid @jonathanolson mentioning you because I suspect that you have the most familiarity with Sim.

[pixelzoom]

I don't see anything in phetsims/joist#409 (mentioned in the Sim.js comment above) that indicates why it's desirable to call stepSimulation( 0 ).

[pixelzoom]

Transferring this issue from diffusion to gas-properties, since the former is a derivative of the latter.

[samreid]

I recall leaving stepSimulation(0) open as a possibility in case some simulation needs to run initialization via stepSimulation(0) before time starts passing. In retrospect, it seems odd that a simulation may need that facility and I would recommend investigating putting dt > 0. For me, this would mean checking dt > 0 instead of dt >= 0 and checking through sims to see if any of them have incorrect startup configurations (hopefully not). We should also check if any simulation is explicitly calling step(0) or equivalent and see if it needs to be dealt with.

[pixelzoom]

That sounds reasonable.

This will need to be resolved before gas-properties begins RC testing. Shooting for week of July 8.

[samreid]

@jonathanolson remarked that step( 0 ) is called a few places in our code and seems reasonable in some places to share code during initialization. Therefore we will change Sim.js to send out strictly >0, but we will allow sims to internally step(0) if necessary.

[samreid]

Leaving self-assigned to make the change in Sim.js

[samreid]

Proposed fix committed, @pixelzoom can you please review?

[pixelzoom]

Proposed fix looks straightforward and correct. But...

In #138 (comment), @samreid said:

... For me, this would mean checking dt > 0 instead of dt >= 0 and checking through sims to see if any of them have incorrect startup configurations (hopefully not). We should also check if any simulation is explicitly calling step(0) or equivalent and see if it needs to be dealt with.

From #138 (comment), it looks like the check for step(0) was done. But was anything done to check for sim with "incorrect startup configurations"?

[samreid]

Testing on Mac Chrome, I'm seeing the first call to stepOneFrame passing through dt approximately equal to 1/60, as expected. I'm no longer sure we need to check initial configurations. I can't see how stepSimulation(0) may be happening on some platforms and somehow correcting the initial configuration (but not on others). I'm happy to close the issue if there's nothing else, or let me know if I'm misunderstanding something.

[pixelzoom]

Thanks for the clarification. Closing.