Speaker ends in a different location after a pulse

[KatieWoe]

Test device Dell Operating System Win 10 Browser Chrome/Firefox Problem description For https://github.com/phetsims/QA/issues/580. Very minor. Happens in published If you manually pulse the speaker, it ends in a slightly different position than where it starts/returns to after reset all.

Visuals

Troubleshooting information:

!!!!! DO NOT EDIT !!!!! Name: ‪Waves Intro‬ URL: https://phet-dev.colorado.edu/html/waves-intro/1.1.0-rc.3/phet/waves-intro_all_phet.html Version: 1.1.0-rc.3 2020-12-08 22:49:34 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36 Language: en-US Window: 1280x658 Pixel Ratio: 1.5/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: 30 uniform: 4096 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 32767x32767 OES_texture_float: true Dependencies JSON: {}

[samreid]

I am able to reproduce the problem on my Mac Chrome. There are 2 factors at play:

1. pulseFiringProperty is set to false at the end of the cycle, but later in the model there is a guard that prevents updating when pulseFiringProperty is marked to false. Correcting this problem alleviates the observable symptoms.
2. dt can overshoot the end of the cycle. I didn't see any frequency values where this yields an observable symptom, but I didn't do an exhaustive test of all frequency values and it seems like it should be corrected at the same time.

Since the fix isn't completely trivial, I'll commit to master only and we can determine if it is working correctly without introducing new problems, and whether it is appropriate to cherry pick to the branch.

[samreid]

@KatieWoe can you please test in master?

[KatieWoe]

Looks fixed on master

[samreid]

@KatieWoe and I discussed whether this should be cherry-picked to the branch, and we agreed that 15-20 minutes more of testing on master (not testing the speaker position itself, but the broader context) would be good to make sure there were no unintended side-effects before we cherry-pick this to the branch.

[KatieWoe]

The sound effect now seems to be out of synch in master. So when you hit pulse, sometimes the sound happens, sometimes it doesn't, and sometimes it happens twice. https://drive.google.com/file/d/1CItJS_E_G84ozA_IDFkGCEBxXSnKHh4l/view?usp=sharing Didn't seem to happen in rc.3, so I think it is connected

[samreid]

Great discovery, thanks for finding this problem! I replicated the problem and will take a look.

[samreid]

In the commit, I adjusted the threshold for sound generation and the problem seems resolved.

I noticed a separate issue where you can stop the continuous sound with the speaker membrane at any position--and this can affect the sound playback, but this problem is less severe and I don't think it will be addressed for this publication. I'll open a side issue for that part (likely at low priority), and reassign this issue for testing.

[samreid]

Ready for feedback on this issue, reassigning to @KatieWoe for testing in master. I think we are still undecided about whether these changes should be cherry-picked to the branch.

[KatieWoe]

The issue looks mostly fixed in master. I think I saw what @samreid mentioned in https://github.com/phetsims/wave-interference/issues/510#issuecomment-758158006. If you stop a wave at as the sound starts, it can mess up the sound when it starts at a new frequency. I have had the wrong frequency start then fix itself, or the sound take a fair amount of time to start. https://drive.google.com/file/d/1N7DIgNoMb2OKR13YHVgrcMhqMSXopjN4/view?usp=sharing

[samreid]

Thanks, I moved the preceding comment to https://github.com/phetsims/wave-interference/issues/512. Can this issue be closed?

[KatieWoe]

Probably, unless you want it checked in an rc. Reopen if you want that @samreid

[samreid]

Reopening to consider whether these changes should be cherry-picked to the branch.

[KatieWoe]

looks good in rc.4