search

phetsims / mean-share-and-balance

"Mean: Share and Balance" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
2 stars 1 forks source link

Consider using rounding in `LevelOutModel` to prevent it from always running #227

Closed jbphet closed 3 months ago

jbphet commented 5 months ago

While working on https://github.com/phetsims/mean-share-and-balance/issues/171, which is for adding sound for the water level changes, I noticed that the water levels sometimes keep adjusting for a very long time after they have visually appeared to reach equilibrium, and in some cases they run indefinitely (as far as I could tell). To deal with this in the sound generator, I had to add a change threshold. It seems like it would be better if the thresholding were in the model.

jbphet commented 3 months ago

I'm looking into this now, and I thought I'd start by checking how long the system really takes to equilibrate. I've run several tests with some debug code in place and timed it, and it's about 9 seconds in most cases, and I haven't seen it go significantly above that yet. This is less time than I originally thought. It may have changed, or it may be that I didn't wait long enough to get a sense for it at the time. I would generally be inclined to simply close this issue now, since 9 seconds doesn't seem to be that bad, except that a separate issue was reported about how many digits are being read out in some cases by screen readers. So, with that in mind, I think I'll spend a little time seeing if rounding the values will work and solve both problems.

Here is a screenshot of the console output from my debug code that logs the water level (waterLevelProperty) for the first notepad cup. This is the last portion prior to when the water level reaches equilibrium:

image

The changes are occurring in the 15th decimal place or further out. I'm going to try rounding to a much smaller number of decimal points to see if it helps.

jbphet commented 3 months ago

I added code to round the water level values to a fixed interval, then was reviewing the behavior with @marlitas over Zoom, and we noticed a problem: When opening and closing the valves, the water levels would often not quite reach the levels that they should (either the mean value when the pipes are open or the table cup value when the pipes are closed). This makes sense. If there is a rounding threshold and the water flow is based on the difference between the cups, that difference will eventually fall below a threshold. This problem existed in the previous versions too (we checked), but was less noticeable because of the non-rounded values.

We decided to try adding code to detect this case, i.e. when the final target levels hadn't been reached but the amount of water flowing was below the rounding threshold. I've done this and, knock on wood, it seems to be working so far.

I think this is ready for review by @marlitas.

marlitas commented 3 months ago

Hmmm looking through https://github.com/phetsims/mean-share-and-balance/commit/d5ace943b3cc0f5517349525fa4a1f872348a3b6 I'm actually wondering if it would be more straightforward to proactively set the water cup value once it hits a certain threshold of closeness. Kind of a close enough just end where we want type logic... @jbphet not sure what your thoughts are here, but I'm happy to explore and chat more tomorrow since it's late in the day.

marlitas commented 3 months ago

@jbphet and I talked through the approach and decided to keep the implementation where we are checking whether water has flowed or not rather than relying on thresholds which seem to be empirical, and would make for harder maintenance in the future.