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

Prevent negative DTs #409

Closed
jonathanolson opened this issue Mar 1, 2017 · 5 comments
Closed

Prevent negative DTs #409

jonathanolson opened this issue Mar 1, 2017 · 5 comments
Assignees

Comments

@jonathanolson
Copy link
Contributor

In phetsims/make-a-ten#273, @phet-steele found it looks like leaving Firefox open with the sim (and leaving overnight) will cause an incorrect ordering of dates when re-opening. This means the sim thinks it has gone back in time, stepping the simulation with a negative dt.

This can also be reproduced by stepping the system clock back in time.

Sims are not built to handle these negative dts, so we should gracefully handle incorrect ordering of dates by:

  1. Don't call stepSimulation with negative dts.
  2. Move back to the "old" time, so the sim will still animate after system clock changes.

Additionally, looking into whether this causes bugs for currently-available sims may be good (@ariel-phet, is this worth time, should this just be fixed in master, or should there be any maintenance releases, particularly for make-a-ten?)

@phet-steele
Copy link

phet-steele commented Mar 1, 2017

This can also be reproduced by stepping the system clock back in time.

@ariel-phet it should be noted that, so far, FF is the only browser that is doing this "naturally". All browsers are affected by manually adjusting the system clock.

Also, no device other than macOS 10.12.3 has been tested at the time of this writing.

@jonathanolson
Copy link
Contributor Author

Fix pushed to master, @phet-steele can you test make-a-ten to ensure that this prevents the bug (using the system clock?)

Fuzzing sims with possibly negative timesteps caused assertion errors in the following: atomic-interactions, energy-forms-and-changes, fluid-pressure-and-flow, gene-expression-essentials, make-a-ten, twixt demo (I'm running a longer fuzz dedicated to this to see if it pops up anything else). Note that this may not catch all bugs, because some sims may not error out (but instead behave incorrectly).

@ariel-phet
Copy link

@jonathanolson I think it would be fine to do a Make-a-Ten maintainence releases (it looks like the credits might need adjusting). But I would just fix in Master and not worry about published sims.

@ariel-phet ariel-phet removed their assignment Mar 1, 2017
@phet-steele
Copy link

phet-steele commented Mar 1, 2017

Chances are github thinks I'm making this comment before this issue existed (my clock is a few days behind)! EDIT: nope, github was smart.

@jonathanolson I think it would be fine to do a Make-a-Ten maintainence releases

@jonathanolson Make a Ten is fixed in current master. I tested with macOS 10.12.3 Chrome & FF, Win 10 Chrome

@jonathanolson
Copy link
Contributor Author

Ok, with it fixed I'll plan on tracking make-a-ten maintenance release in phetsims/make-a-ten#273.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants