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

Re-think the Auto Scale feature in the Discrete screen. #139

Closed pixelzoom closed 3 years ago

pixelzoom commented 3 years ago

Over in https://github.com/phetsims/fourier-making-waves/issues/36, we've been discussing where to put the y-axis zoom buttons. It's been an on going discussion, as it was back in the Java design days. The reason we need the zoom buttons is to allow the student to change the y-scale when Auto Scale is off.

I think we should take a big step back, and reconsider what we're doing here. The y-zoom buttons and Auto Scale checkbox were a solution to a problem. I think we should take another look at that problem, find a better solution, and consider ditching the y-zoom buttons and Auto Scale checkbox.

If I recall correctly, the Java design team added the ability to turn Auto Scale on/off for these reasons:

(1) With Auto Scale on, the user might not notice the change in y-axis scale. The thinking is that if they have to turn if on (and they understand what "Auto Scale" means and applies to) then they are more likely to notice the y-scale change. This was the primary concern, because auto scaling can (in general) be something that is confusing and needs to be scaffolded. But I question whether the current design is effective here. It's a a big leap to go from the "Auto Scale" label to "this automatically changes the scale of the y-axis to fit the Sum waveform, so while this is checked the y-axis will be changing".

(2) With Auto Scale off, the user can zoom in to see specific parts of the Sum waveform. This was a secondary concern. I question if it's necessary, and if zoom is used this way. If we weren't concerned about (1), I'm pretty sure we would ditch the y-zoom buttons -- as we did in the other 2 screens.

So let's say that (1) is still the primary concern, and (2) can be dropped. If we ditched the y-zoom buttons and Auto Scale checkbox, and made the Sum chart always autoscale, how could we make the y-scale change more obvious? Some options/ideas, not mutually exclusive:

(a) Add veritical dimensional arrows to the Sum graph, showing the An range of [-1.5,1.5]. As the y-scale changes, the size of these dimensional arrows will change. Locate the dimensional arrows towards the left edge of the Sum chart, close to the y-axis.

(b) Similar to (a), but a different way of showing the range of An. Add special grid lines to the Sum chart, at -1.5 and 1.5, the range of An. Use a different color for these grid lines. Maybe even label their values on the y-scale -- but that could get messy when the Sum's amplitude is close to 1.5, as tick labels could start to overlap.

(c) On the Sum chart, show small bars for each harmonic's amplitude, color-code to the harmonic colors. As you change an amplitude slider, its small bar on the Sum chart would also change. This would show you how the amplitude of the Sum relates to the amplitudes of the individual harmonics. As the Sum's y-scale changes, the size of these bars would change. Locate the bars towards the left edge of the Sum chart, close to the y-axis.

(d) Change how we handle y-axis tick marks for the Sum chart. Make it behave like the Sum chart in the Wave Packet screen, where we label only the min/max. This would make it a more obvious that the y-axis scale is changing, as I think it makes the change more noticeable in the otehr 2 screens. And it would have a bonus - you could see what the sum's max amplitude is. That's something we don't do now, which is odd considering that amplitudes are quantitative.

Finally, a note about implementation... A confusing implementation is often a reflection of a confusing UI design. And that's certainly the case with the y-zoom buttons and autoscale checkbox. The implementation for the other 2 screens is so much cleaner, because the UI is cleaner. I'm not saying that implementation should be a primary driver of UI design. But I am saying that in this case, problems with the implementation are a reflection of problems with the UI design.

@arouinfar @ariel-phet @kathy-phet thoughts?

pixelzoom commented 3 years ago

In https://github.com/phetsims/fourier-making-waves/issues/105#issuecomment-889386852, @arouinfar met with an instructor who uses this sim, and reported:

I met with Steve, but didn't hear back from Mike. Steve has used this sim in the past for the Sound and Music course. He also mentioned using it in mechanics (classical and quantum) courses when Fourier transforms come up.

Maybe ask Steve about the Auto Scale feature in the Discrete screen? Does he mention it to his students? Does he have them leave it on or off? When it's on, does he see any problems with students noticing the y-scale, or understanding the Sum chart? Does this feature or any of these issues even come up?

ariel-phet commented 3 years ago

@pixelzoom I would certainly be fine with the sum always auto scaling.

  1. Perhaps we could actually just add those words to the title -- "Sum (Auto Scaled)" I think we have plenty of room for that

  2. I would go with your idea of color coding some grid lines (or maybe even just clearly color coding the tick marks on the left). Currently the background grid changes pretty obviously when autoscaling.

Basically, I think with a tweaked title and some minor visually enhanced hints we could just have everything automatically auto scale. Really the learning goal is about the waveform shape and how it is an addition of multiple waves, the exact quantitative scale is less important in my opinion as long as student are recognizing the idea of aspect of the components adding constructively and destructively.

And thanks for the revisit of the design issue!

pixelzoom commented 3 years ago

By the way... I labeled this as high-priority because a big task that I still have to do is cleanup how the y-scale is handled throughout the code. And that task has to be done pronto. All of the charts in this sim are based on a base class that handles x & y axis scaling. For the y axis, that scaling is quite the mess right now. Some screens use the built-in auto scaling, while other screens ignore those features and "bolt on" on y-zoom buttons and user-controlled scaling. Lots of problems (and lots of work!) will disappear if we simplify the UI for y-scale.

arouinfar commented 3 years ago

I would be fine with autoscaling the y-axis of the Sum graph on all screens. Manually zooming makes it explicitly clear that the y-axis scaling differs from the Amplitude/Harmonics graphs. However, I do not think that it is critical to any learning goal in the simulation, and the topic is advanced enough that the audience should have some basic graph literacy.

A possible downside of autoscaling is that a user may not notice the difference in scales. Styling +/-1.5 value or graph lines differently could be a nice touch, but it won't work for all zoom levels, as the most extreme case is [-10, 10].

Perhaps we could actually just add those words to the title -- "Sum (Auto Scaled)" I think we have plenty of room for that

I think this is the best option. It's perhaps not the most elegant-looking solution, but I think it will be the most functional.

@pixelzoom let's always autoscale the y-axis of the Sum graph and change its title to "Sum (Auto-Scaled)".

pixelzoom commented 3 years ago

@pixelzoom let's always autoscale the y-axis of the Sum graph and change its title to "Sum (Auto-Scaled)".

I'm sorry, but this is gross. And are we going to do this in both Discrete and Wave Game? And for all 3 charts that auto-scale in Wave Packet?

(EDIT: But I could learn to ignore it if it means that the Auto Scale checkbox and y-zoom buttons go away :)

arouinfar commented 3 years ago

Here's another idea, perhaps we can use the density of grid lines to indicate the zoom level (labelling only min/max and zero). We do something similar with the histograms in Gas Properties, and I think it works well.

pixelzoom commented 3 years ago

Density of grid lines is a great idea!

@arouinfar and I discussed a few loose ends on Zoom. In the Discrete and Wave Game screens, we will do this for the Sum chart:

pixelzoom commented 3 years ago

@arouinfar @ariel-phet FYI... I'm partway through this, and about to take a break for a few days. So I published 1.0.0-dev.44 as a sort of check point. The y-axis scaling and gridlines are not exactly as we discussed - but maybe good enough? In any case, it's in a state where you can test-drive it to see how you like it without the y-zoom buttons and Auto Scale checkbox. I think it works nicely, a big improvement.

ariel-phet commented 3 years ago

@pixelzoom I would agree it is an improvement, and a good direction.

I am curious why you think "Sum (Auto-Scaled)" or some other label of that nature is gross. I am not tied to those words exactly, but I do think if there is something we can put in the title that conveyed that idea cleanly it would be helpful. It seems some simple word in the title could do a lot of lifting. I see it something like a label on a financial graph that says "Dollars (inflation adjusted)" or such.

pixelzoom commented 3 years ago

I am curious why you think "Sum (Auto-Scaled)" or some other label of that nature is gross. ...

While we have been discussing mainly the Discrete screen's "Sum" chart, it is not the only chart that auto scales. There are a total of 5 charts whose y-axis auto scales (1 in Discrete, 1 in Wave Game, all 3 in Wave Packet). All of those charts have the same concerns about whether the user will notice the y-scale change. So I don't see why we would title the Discrete chart "Sum (Auto-Scaled)" and not do something similar for the other 5.

Secondarily, "Auto-Scaled" is a tad inaccurate/unclear. The x-axis does not auto scale, only the y-axis does. So something like "y-axis Auto-Scaled" would be more informative. And yes, we could probably still get away with "Auto-Scaled", if that were the only concern.

What's "gross" imo is labeling 5 charts with "Blah Blah (Auto-Scaled)" or worse "Blah Blah (y-axis Auto Scaled)". That is super ugly. The longest title becomes "Amplitudes of Fourier Components (Auto-Scaled)". And if we have to do that for 5 charts, then imo we have a design problem that should be addressed in some other way.

kathy-phet commented 3 years ago

I would suggest leaving it as "Sum" and then seeing if there are interpretation issues in the interviews - noticing this change.

pixelzoom commented 3 years ago

I've created https://github.com/phetsims/fourier-making-waves/issues/150 to look for auto-scaling problems in interviews.

ariel-phet commented 3 years ago

Just throwing one more thought out there (if interviews reveal any issues)

We could have a smallish clickable icon (similar to our info button) that if clicked just shows a barebones dialog that says "Note, the y-axis in this graph is auto scaled" -- could even be an info button next to the y-axis label. Not suggesting we implement anything like that currently, just marking down an idea if we feel some clarification is needed (that is less "gross" :) )

pixelzoom commented 3 years ago

If interviews reveal any issues... Rather than changing the chart titles to (e.g) "Sum (Auto-Scale)", we could consider changing the y-axis labels to "Amplitudes (Auto-Scaled)". Since it's the y-axis that's scaled, not the x-axis, this might be more helpful. And the label would be identical for all charts that have an auto-scaled y axis.

pixelzoom commented 3 years ago

In https://github.com/phetsims/fourier-making-waves/issues/139#issuecomment-900727899, I asked:

I'm partway through this, and about to take a break for a few days. So I published 1.0.0-dev.44 as a sort of check point. The y-axis scaling and gridlines are not exactly as we discussed - but maybe good enough? ...

@arouinfar and I reviewed on Zoom, and we like the way this is currently working. In the Discrete screen, this is a preferrable strategy for grid lines and tick marks. It's not important to show the peak amplitude value, and doing so (as a grid line and tick mark) would collide with the 1-unit grid lines and tick marks.

Assigning to @arouinfar to review. If it looks good, please close this issue.

arouinfar commented 3 years ago

I did not observe any issues with the autoscale behavior in interviews, so I think we can leave things as-is.