phetsims / fourier-making-waves

"Fourier: Making Waves" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 3 forks source link

How should y-axis autoscale for Amplitudes chart? #118

Closed pixelzoom closed 3 years ago

pixelzoom commented 3 years ago

In the Java and HTML5 versions...

With these settings, the top chart shows 1 bar at 12pi:

screenshot_1129

If I then move the Wave Packet Center to either 9pi or 15pi, I see no bars in the top chart. For example:

screenshot_1128

I don't understand this case. It occurs in both the Java and HTML5 versions. Is this a bug? Or is there an explanation?

pixelzoom commented 3 years ago

Oh wait.... I do see 2 bars in the bottom screenshot, at 8pi and 10pi. They are so small that I missed them. I still don't understand why, but I have a little more confidence that this is correct :)

@arouinfar is this indeed correct? Do you think it will be a problem for users? Should we be using something other than grayscale, so that even tiny bars would be more visible?

pixelzoom commented 3 years ago

Ah.... I think the problem here is with the autoscaling of the y axis. Those 2 bars that are hard to see should be made bigger by autoscaling the y axis, and that's not happening. Self assigning to investigate.

pixelzoom commented 3 years ago

The y-axis scale is indeed correct -- it's scaled to the Continuous Waveform, whose max amplitudes is ~2.5, see screenshot below. And you can barely see the component amplitudes at 8pi and 10pi.

Should the y-axis be scaled to the maximum amplitude for whichever plots are visible? That would mean that the chart's scale will likely change when toggling the "Continuous Waveform" checkbox.

@arouinfar thoughts?

pixelzoom commented 3 years ago

In the above commit, I've changed the auto-scale behavior of the Amplitudes chart. Test drive in master or 1.0.0-dev.37.

The good news is that the tiny amplitudes that were so difficult to see in https://github.com/phetsims/fourier-making-waves/issues/118#issue-961161859 are now clearly visible:

screenshot_1137

The bad news is that there are now some situations where the size of the amplitude bars doesn't appear to change, but the y-axis scale is changing dramatically. This tends to occur when there are 1 or 2 amplitudes bars (max component spacing).

To reproduce, start with these settings:

k1 = 2pi k0 = 12pi σk = 1

You'll see this Amplitudes chart. Note the y-axis scale.

screenshot_1139

Keeping your eye on the y-axis scale, slide the Wave Packet Center (k0) slider around. You'll see setting where there is a dramatic change in the y-axis scale, and no change in the amplitude bar height. For example, here's k0 = 35.3 (11.25pi):

screenshot_1140

This seems better to me than the alternative, which was not seeing the tiny amplitude bars. The auto-scaling in this screen is something that definitely needs to be highlighted in the Teach Tips, and maybe that will be sufficient.

@arouinfar please review. Let me know what you think, whether you want to change anything, etc.

pixelzoom commented 3 years ago

In the "to reproduce" scenario in the previous comment... While you're dragging the Wave Packet Center (k0) slider, note that there is similar behavior for the Components and Sum charts. That is, there are settings where the waveforms don't appear to change much (or at all) and the y-axis scale is changing dramatically. So the behavior in the Amplitudes chart is now consistent with those charts.

pixelzoom commented 3 years ago

Going back to the example that I described in my first comment, https://github.com/phetsims/fourier-making-waves/issues/118#issue-961161859, where I couldn't see the amplitudes at 8pi and 10pi... Note that when you check the "Continuous Waveform" checkbox, seeing those amplitudes is still an issue -- AND the y-axis scale changes dramatically as you toggle that checkbox.

To reproduce:

  1. set k1 = 2pi
  2. set k0 = 9pi
  3. set σk = 1
  4. Toggle the "Continuous Waveform" checkbox on and off.

The Amplitudes chart is now auto-scaled based on which charts are visible. And there's a huge difference in the y-axis range for the component amplitudes and the Continuous Waveform. E.g.:

Continuous Waveform off:

screenshot_1141

Continuous Waveform on:

screenshot_1142
pixelzoom commented 3 years ago

Note that this issue also applies to the Sum chart. When "Waveform Envelope" is checked, it's maximum amplitude may be greater than the sum's maximum amplitude.

To demonstrate, set to these values:

screenshot_1156

With "Waveform Envelope" unchecked:

screenshot_1157

With "Waveform Envelope" checked, note the different y-axis scale:

screenshot_1158
pixelzoom commented 3 years ago

Because of changes in https://github.com/phetsims/fourier-making-waves/issues/158, we now want to always scale the Amplitudes chart to the Continuous Waveform. Self-assigning to change this.

pixelzoom commented 3 years ago

Done in the above commit - the Amplitudes chart now scales to the peak amplitude of the Continuous Waveform, regardless of whether it's visible.

@arouinfar please review in master.

arouinfar commented 3 years ago

Looks good, thanks!