phetsims / calculus-grapher

"Calculus Grapher" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
4 stars 4 forks source link

Scaling of graphs seems inconsistent #302

Closed KatieWoe closed 1 year ago

KatieWoe commented 1 year ago

Test device Samsung Operating System Win 11 Browser Chrome Problem description For https://github.com/phetsims/qa/issues/921 d/dx of sin(x) is cos(x) and d/dx of cos(x) is -sin(x). When you make a sin function on the Lab screen, and look at it's first and second derivatives, the shapes are right but either the amplitude or the scaling of the graphs seem inconsistent. Unless I am misremembering, when the three graphs are at the same level of zoom the lines should have the same amplitude. This is not the case in the default zooms. Additionally, if you adjust the zoom so that the sin and -sin graphs are equal amplitudes and make sure the cos graph is at the same zoom, it will still not have the same amplitude as the other two graphs. Steps to reproduce

  1. On the Lab screen, create a sin wave
  2. Select the option to view the second derivative
  3. Adjust the zoom of the second derivative graph so it is the same size as the sin wave
  4. Adjust the zoom of the first derivative the same amount.

Visuals These graphs at default zoom levels: nozoomchange Adjusted: scaleinconsistant What these three graphs should look like according to Desmos: desmosremember

Troubleshooting information:

!!!!! DO NOT EDIT !!!!! Name: ‪Calculus Grapher‬ URL: https://phet-dev.colorado.edu/html/calculus-grapher/1.0.0-dev.25/phet/calculus-grapher_all_phet.html Version: 1.0.0-dev.25 2023-03-20 03:37:03 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/111.0.0.0 Safari/537.36 Language: en-US Window: 1536x714 Pixel Ratio: 1.25/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: 31 uniform: 4096 Texture: size: 8192 imageUnits: 32 (vertex: 32, combined: 64) Max viewport: 8192x8192 OES_texture_float: true Dependencies JSON: {}
veillette commented 1 year ago

Yes, that's an interesting observation. I wonder if we should rethink our scaling to get that effect. There is nothing incorrect per say in the simulation as the sinusoidal is not necessarily sin(x) but sin(k x) and that k will affect the derivative, which will become k cos(k x). The same thing applies to the second derivative. However, maybe we can set the default width, which is the wavelength, to be 2pi. However , we wanted to be consistent across all functions such that we would have by default a very wide parabola, triangles, etc.

pixelzoom commented 1 year ago

I don't have an opinion on this one, will defer to @veillette.

veillette commented 1 year ago

Currently the "width" of the parabola and the triangle (when the peak is at its half y max), matches the wavelength of the sinusoid Calculus Grapher screenshot (52)

The current maximum wavelength is set to 5, and by default, (by having the slider set to the third tick is 3), whereas to have the effect that @KatieWoe proposed the wavelength by default should be 6.28 (2*pi). Calculus Grapher screenshot (53)

One way around it is to have the width of the slider tune to half the wavelength, rather than the wavelength, this would bring us much closer to the effect that @KatieWoe was expecting.

veillette commented 1 year ago

There are two changes that need to be done to bring us to this effect.

(I turned on the values in the screenshot below to give you an idea of scale. I think @KatieWoe ' point was that it would be nice, if without values, and with the default setting, the derivative of the sine function would all have the same (nearly) amplitude.

Calculus Grapher screenshot (57)

veillette commented 1 year ago

Here is the patch if we want to go down that road.

Subject: [PATCH] change default values for sinusoid and slider position
---
Index: js/common/CalculusGrapherConstants.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/CalculusGrapherConstants.ts b/js/common/CalculusGrapherConstants.ts
--- a/js/common/CalculusGrapherConstants.ts (revision ff903f8238002dffe7764bb8ab46e98355947bcc)
+++ b/js/common/CalculusGrapherConstants.ts (date 1679409735505)
@@ -75,7 +75,7 @@
   CURVE_MANIPULATION_WIDTH_RANGE: new RangeWithValue(
     0.1 * CURVE_X_LENGTH,
     0.5 * CURVE_X_LENGTH,
-    0.20 * CURVE_X_LENGTH ),
+    0.30 * CURVE_X_LENGTH ),

   // model height associated with curveManipulationDisplay (in the same unit as x-Range)
   CURVE_MANIPULATION_Y_RANGE: new Range( -0.25, TYPICAL_Y + 0.25 ),
Index: js/common/model/TransformedCurve.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/TransformedCurve.ts b/js/common/model/TransformedCurve.ts
--- a/js/common/model/TransformedCurve.ts   (revision ff903f8238002dffe7764bb8ab46e98355947bcc)
+++ b/js/common/model/TransformedCurve.ts   (date 1679409568693)
@@ -480,7 +480,7 @@
     const closestPoint = this.getClosestPointAt( x );

     // Wavelength associated with the sinusoidal function
-    const wavelength = width;
+    const wavelength = width * 2;

     // Cosine function to apply to points. Cosine function passes through `position`
     const cosineFunction = ( x: number ) =>
amanda-phet commented 1 year ago

Thanks for bringing this up @KatieWoe

Discussed with @pixelzoom and @veillette and we'd like to divorce the width of the sine wave from the other curves, so we can have a default width of 2π.

veillette commented 1 year ago

In the commit above, the sinusoid wavelength/width is set to be 2pi by default.

With the default width value, we have

Calculus Grapher screenshot (59)

at the narrowest setting we have a couple of waves, enough to see the periodic behavior with multiple wavelengths..

Calculus Grapher screenshot (60)

and the widest is not so useful but I don't see any way around it.

Calculus Grapher screenshot (61)

veillette commented 1 year ago

I think this is a decent compromise. The students are more likely to use the default values in their exploration, and therefore I am not very worried about the usability at the very wide setting.

I'm assigning to @amanda-phet to review.

amanda-phet commented 1 year ago

This is looking great to me! This is a really good change. Closing this issue.