phetsims / curve-fitting

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

Wrong line of best fit appears when adjusting points at (0,10) and (0,-10) #162

Closed loganbraywork closed 5 years ago

loganbraywork commented 5 years ago

Report a Problem Instructions: Copy the form below and paste it into an email addressed to phethelp@colorado.edu. Please fill out the form as accurately and concisely as you can.

Test device Windows 7 Laptop Operating System Professional 6.1 Browser Chrome version 77.0.3865.75 Problem description From https://github.com/phetsims/QA/issues/423

When placing points in a vertical line at the same exact x value, if points are placed at the edges (y = 10 & y = -10), then adjusted and replaced, the equation and line of best fit will show up as incorrect. It should appear/normally appears as a horizontal line through the middle point, but instead appears as a not quite vertical line. Steps to reproduce

  1. Check both the "Values" and the "Curves" boxes
  2. Place 3 points at (0,0), (0,10), and (0,-10)
  3. Adjust both the point at (0,10) and (0,-10) so that they are at any coordinates besides (0,10) and (0,-10)
  4. Return the points moved in the above step back to their original positions at (0,10) and (0,-10) Visuals 2019-9-11LneoBstftCurvefit

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Curve Fitting‬:‪Curve Fitting‬ URL: https://phet-dev.colorado.edu/html/curve-fitting/1.0.0-dev.19/phet/curve-fitting_all_phet.html?stringTest=double Version: 1.0.0-dev.19 2019-09-04 19:08:06 UTC Features missing: touch User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/77.0.3865.75 Safari/537.36 Language: en-US Window: 1366x576 Pixel Ratio: 1/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: 32 varying: 32 uniform: 256 Texture: size: 8192 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 8192x8192 OES_texture_float: true Dependencies JSON: {}

amanda-phet commented 5 years ago

This kind of looks like a rounding error. When you move the bottom point to (0, -10) it looks like it's not exactly on the y-axis, but maybe a pixel to the left. Hard to say for sure, but my best guess in this situation is that the coordinates have been rounded and the points might not be as precise as we think.

SaurabhTotey commented 5 years ago

I believe this is a rounding issue as @amanda-phet suggests. The points have a more precise position with a high amount of decimals compared to the value display which only shows values up to the tenths place. I am unable to reproduce this issue with the snapToGrid=true query parameter.

However, it may be apt to round all position values to the nearest tenths place to avoid confusion from things like this. @amanda-phet, would it be worth making points round their position values so that they are where they say they are?

SaurabhTotey commented 5 years ago

I pushed a temporary commit to master where the points' positions are rounded to the nearest tenth. @amanda-phet let me know whether to keep this commit or whether to revert it.

amanda-phet commented 5 years ago

I pushed a temporary commit to master where the points' positions are rounded to the nearest tenth.

Does this mean the points snap to the position on the graph? That's how it seems to me, and I find that behavior a bit odd. My coordinates say (0,10) and I drop the point, then the point moves a few pixels up. If that's what is happening, I think I prefer the original.

amanda-phet commented 5 years ago

@KatieWoe what do you think of the change?

KatieWoe commented 5 years ago

It is definitely a noticeable change. It didn't personally bother me, but might have felt better if I could see the grid it was attaching too. Overall, I don't think the original bug is too bad, so I would be happy either leaving as is or with the fix @SaurabhTotey proposed.

amanda-phet commented 5 years ago

My vote is to leave as-is, and recognize that this rounding issue could happen, but is unlikely since the sim is not intended for such precise usage.

SaurabhTotey commented 5 years ago

Ok, sounds good. I'll revert the change then.

SaurabhTotey commented 5 years ago

The changes have been reverted. Closing this issue.