phetsims / hookes-law

"Hooke's Law" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 4 forks source link

Model and decimals in sim screens #57

Closed kathy-phet closed 3 years ago

kathy-phet commented 6 years ago

This issue is broken off for a separate discussion reflecting on Hooke's law screens. Below is the initial context that initiated the discussion. But in the next comment I will try to summarize what I am seeing as the differences in the screens and the place value needs.

@kathy-phet wrote:

(Question for Amy): Re significant digits. In playing with this screen and thinking about taking data, I think we need to look even deeper at the significant digits. I like the new snappiness of the position control. Does it need to have 3 digits in this control? Also an issue with three digits in this control is that force is only read out to two digits - so it does not have enough sig figs to register the change from 0.350 to 0.351 or 0.034 in position. There is a fundamental change in this particular screen because we are now adjusting position, not force as in the prior screens.

@pixelzoom replied:

The number of decimal places was carefully chose so that no controls or interactions feel to “coarse”. If you reduce the number of decimal places in displacement (for example), the applied force is going to move in big jumps in some of the screens. Also beware that this sim allows changing all 3 variables in the equation F = kx. So the number of decimal places need to be compatible across all quantities, or we will end up with loops in the model — which I spent 8+ hours investigating and fixing for displacement only today. And “loops” were never an issue until we had to consider the PhET-iO data stream. Which is related to your next bullet.

pixelzoom commented 6 years ago

Specifically, this issue was broken off from https://github.com/phetsims/phet-io-wrapper-hookes-law-energy/issues/2#issuecomment-396770874.

pixelzoom commented 6 years ago

@kathy-phet also note that there's a more general issue that discusses the topic of "number of decimal places" as related to the PhET-iO message stream. See https://github.com/phetsims/phet-io/issues/1332.

kathy-phet commented 6 years ago

Here is a summary of how I am thinking about these screens.

Screen 1: Here the primary controls are F and k. Displacement is the responding to these slider values. F is constrained to an integer (1,2,3,.... ) k is constrained to the tens value (100, 110, 120, ...) Displacement should be the calculated value from x = F/k. And x value needs 3 decimal places because these nice F and k values can divide in such that 3 decimals are needed to capture the value well enough to graph and show the relationship. Model value will have many decimal places ... displayed value will have 3 decimal places.

One model complication I don't fully follow on this screen is that position is adjustable with the arm, but F should always be the exact integer value shown since it is on the slider and that is exact. So I am not sure how the model is working here.

Screen 2: Similar behavior and decimal needs as to Screen 1.

Screen 3: Here we are controlling different parameters, and so what is the dependent and independent variables have changed. x is now determined by our control all the time (whether slider or arm), and is constrained to be nice steps so as to make it easy to do a controlled experiment (this is pedagogically very useful). It should never need to be anything other than what it is set at and should not be calculated from F and k. x really only needs 2 decimal places here 0.20. (snappy by 0.05, tweaky by 0.01) x in the display can be 0.20. x in the model is set to be exactly 0.2000000 (or however many digits there are), but it is not calculated and cannot change.

k is also similarly defined with a set number of digits (100, 110) should not need to be calculated.

F is now the dependent term and should be directly calculated from k and x. F=kx. If k can be 110 and x can be 0.21, F is not expected to be a "nice integer number" and it would be best if it is displayed with 3 digits. F=XX.X in the display. F in the model has many digits.

I guess I am seeing 2 different model variants depending on the screen - one where x=F/k and one where F=kx. How does the third screen work right now? where F is now the calculated value, not the controlled/set value?

Apologies for not having much bandwidth to discuss this today - but I wanted to put these thoughts down for Amy and Christ to look over.

arouinfar commented 6 years ago

Displacement definitely needs three decimal places on all three screens, otherwise some interactions will feel coarse, as @pixelzoom described.

We consistently display the same number of decimal places across all screens. I believe we made the decision of how many decimals to display was based on the interaction pattern of the first two screens (user-controlled F and k).

@kathy-phet raises some really great points about the third screen. I do not think there is any pedagogical advantage to controlling x in 0.001 m steps. We could kill the third decimal place altogether, and have the thumb/arm snap to 0.05 m steps, and use the tweaker to adjust by 0.01 m.

When x is small, increasing k results in F increasing in jumps, regardless of whether x has two or three decimals. Since F is the derived quantity on this screen, it would benefit from an additional digit.

I would vote for:

@pixelzoom I'd m also curious about this question:

I guess I am seeing 2 different model variants depending on the screen - one where x=F/k and one where F=kx. How does the third screen work right now? where F is now the calculated value, not the controlled/set value?

On the Energy screen, does the model calculate F from the user-controlled x and k?

pixelzoom commented 6 years ago

@arouinfar asked:

On the Energy screen, does the model calculate F from the user-controlled x and k?

Intro and Systems screens: Changing x solves for F. Changing F or k solves for x. Energy screen: Changing x or k solves for F. Changing F solves for x.

pixelzoom commented 6 years ago

@arouinfar wrote:

• Leave sig figs on Intro & Systems as they are

The current implementation uses the same sig figs throughout all screens. Changing so that one screen (Energy) can be different will be nontrivial.

• On Energy, reduce displacement to two decimals. • On Energy, display Applied Force to the nearest tenths place.

As written, this seems to suggest that we constrain displacement in the model, but constrain applied force only in the view ("display"). Is that what you intended? I want to be very clear that model vs view constraint are very different things. Constraining anything in the model typically leads to more floating-point errors.

kathy-phet commented 6 years ago

Energy screen: Changing x or k solves for F. Changing F solves for x.

Why do you need a changing F solves for x on this screen? x is fixed by the slider/model value?

arouinfar commented 6 years ago

Energy screen: Changing x or k solves for F. Changing F solves for x.

@pixelzoom it's unclear to me how a user could directly change F. The user can move the arm (which changes x) or adjust the k or x sliders.

The current implementation uses the same sig figs throughout all screens. Changing so that one screen (Energy) can be different will be nontrivial.

An alternative would be:

As written, this seems to suggest that we constrain displacement in the model, but constrain applied force only in the view ("display"). Is that what you intended?

I'm not sure I completely follow @pixelzoom. Doesn't the Displacement slider set the value of the x in the model? The Applied Force is then calculated, and its displayed value is rounded to the nearest tenths place.

pixelzoom commented 6 years ago

About the Energy screen...

@kathy-phet asked:

Why do you need a changing F solves for x on this screen? x is fixed by the slider/model value?

@arouinfar said:

I'm not sure I completely follow @pixelzoom. ...

Sorry, let me try explaining again.

All screens use the same model code. In the model, changing x computes F; changing F computes x. The model has an option that indicates which parameter is computed when k changes. In the first 2 screens, changing k computes x. In the Energy screen, changing k computes F.

In the Energy screen, there is no direct control of F, so the "changing F computes x" part of the model is not exercised. But it's still part of the model code.

pixelzoom commented 6 years ago

@arouinfar asked:

I'm not sure I completely follow @pixelzoom. Doesn't the Displacement slider set the value of the x in the model? The Applied Force is then calculated, and its displayed value is rounded to the nearest tenths place.

Correct, the displacement slider and robotic arm are both constrained to set x to values with at most 3 decimal places. So when interacting with the displacement slider or robotic arm, x values will have 3 decimal places in the model.

But values computed by the model (e.g. F is computed when dragging the robot arm changes x) are not similarly constrained to any number of decimal places, and computed values may contain floating-point error. If you want to constrain all model values to the same number of decimal places shown in the view, that is different (and more difficult) than simply controlling the number of decimal places used by view components to control and display values.

kathy-phet commented 6 years ago

I don’t want the model to display fewer decimals nor for PhET-io model values to be fewer decimals, but when the slider is set to 0.130, can you not have that set the model to 0.130000000 exactly? So all model position values have decimals that are 0 down the line? I agree F would have some non zeros down at the smallest decimals because of calculations, but not sure why x would need to.

Kathy

Sent from my iPhone

On Jun 14, 2018, at 6:35 PM, Chris Malley notifications@github.com<mailto:notifications@github.com> wrote:

@arouinfarhttps://github.com/arouinfar asked:

I'm not sure I completely follow @pixelzoomhttps://github.com/pixelzoom. Doesn't the Displacement slider set the value of the x in the model? The Applied Force is then calculated, and its displayed value is rounded to the nearest tenths place.

Correct, the displacement slider and robotic arm are both constrained to set x to values with at most 3 decimal places. So when interacting with the displacement slider or robotic arm, x values will have 3 decimal places in the model.

But the value computed by the model (e.g. F when dragging the robot arm) is not similarly constrained to any number of decimal places, and the computed value may contain floating-point error. If you want to contain all model values to the same number of decimal places shown in the view, that is different (and more difficult) than simply controlling the number of decimal places used by view components to control and display values.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/phetsims/hookes-law/issues/57#issuecomment-397478962, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AE3FZB2QjYYeMkNCTeqiKwiX9eBfIiwjks5t8wE-gaJpZM4UmnOo.

pixelzoom commented 6 years ago

I don’t want the model to display fewer decimals nor for PhET-io model values to be fewer decimals, but when the slider is set to 0.130, can you not have that set the model to 0.130000000 exactly?

UI controls do set their associated Properties to exact values. But as I stated above: ... values computed by the model (e.g. F is computed when dragging the robot arm changes x) are not similarly constrained to any number of decimal places, and computed values may contain floating-point error.

I agree F would have some non zeros down at the smallest decimals because of calculations, but not sure why x would need to.

If you drag a control that changes F, x will be computed. And depending on the values of F and k, x may contain some floating point error.

In Javascript: 0.1 + 0.2 = 0.30000000000000004 0.05 + 0.01 = 0.060000000000000005 Really.

kathy-phet commented 6 years ago

On the third screen can we disable the recalculation of x. When k changes only F should change. x should not be calculated or changed on this screen, it should be fixed by the slider position only.

Kathy

Sent from my iPhone

On Jun 18, 2018, at 12:43 PM, Chris Malley notifications@github.com<mailto:notifications@github.com> wrote:

don’t want the model to display fewer decimals nor for PhET-io model values to be fewer decimals, but when the slider is set to 0.130, can you not have that set the model to 0.130000000 exactly?

UI controls do set their associated Properties to exact values. But as I stated above: ... values computed by the model (e.g. F is computed when dragging the robot arm changes x) are not similarly constrained to any number of decimal places, and computed values may contain floating-point error.

I agree F would have some non zeros down at the smallest decimals because of calculations, but not sure why x would need to.

If you drag a control that changes F, x will be computed. And depending on the values of F and k, x may contain some floating point error.

In Javascript .05 + .01 = 0.060000000000000005. Really.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/phetsims/hookes-law/issues/57#issuecomment-398155126, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AE3FZB56AosfgPdASV2vBYfK_i90z2Gaks5t9_TGgaJpZM4UmnOo.

pixelzoom commented 6 years ago

In case anyone is interested in where Javascript floating-point error comes from... This is the simplest explanation that I've been able to find: https://modernweb.com/what-every-javascript-developer-should-know-about-floating-points/.

pixelzoom commented 6 years ago

@kathy-phet I'm not at all clear on what problem (specifically) you're trying to solve.

pixelzoom commented 6 years ago

When a NumberProperty value in the model changes, its value is now printed to the browser console if you use the ?log query parameter.

There are 2 NumberProperties that (when computed) contain floating-point error that makes them have more decimal places than desired. Those 2 Properties are appliedForceProperty (F) and leftProperty (position of the left end of the robotic arm). So I'm perplexed about why we need to "disable the recalculation of x" in the Energy screen when I see no such recalculation of displacement occurring (that was addressed in https://github.com/phetsims/hookes-law/issues/52#issuecomment-396767284) and all values for displacement (in all screens) appear to have the desired number of decimal places (max 3).

kathy-phet commented 6 years ago

I will look again. Goal was to stop the recursion but if it is not being recalculated it should be fine. I thought I saw recursion still when using position tweakers but I will look again when I can. Are the position tweakers at 0.01 steps now? K

Sent from my iPhone

On Jun 18, 2018, at 6:11 PM, Chris Malley notifications@github.com<mailto:notifications@github.com> wrote:

When a NumberProperty value in the model change, its value is now printed to the browser console if you use the ?log query parameter.

There are 2 Properties that (when computed) contain floating-point error that makes them have more decimal places than desired. Those 2 Properties are appliedForceProperty (F) and leftProperty (position of the left end of the robotic arm). So I'm perplexed about why we need to "disable the recalculation of x" when I see no such recalculation of displacement occurring (that was addressed in #52 (comment)https://github.com/phetsims/hookes-law/issues/52#issuecomment-396767284) and all values for displacement appear to have the desired number of decimal places (max 3).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/phetsims/hookes-law/issues/57#issuecomment-398233927, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AE3FZNJMpZPM_HQYdVXKDUoBmgxOdy01ks5t-EG3gaJpZM4UmnOo.

pixelzoom commented 6 years ago

I really think we should table this issue until we can have a real discussion. Getting fragmented/unclear feedback based on no specific version is making me a little crazy and does not feel productive.

pixelzoom commented 6 years ago

@kathy-phet asked:

Are the position tweakers at 0.01 steps now?

No. Displacement tweakers on the Energy screen are still at 0.001 m increments. I can change it (relatively) easily for the tweakers, but I didn't think we had reached a consensus yet.

pixelzoom commented 6 years ago

@arouinfar and @kathy-phet had a phone conversation, then @arouinfar and I had a phone conversation. Here are the requested changes:

All screens:

Intro & Systems screens:

Energy screen:

@arouinfar is that correct?

arouinfar commented 6 years ago

That's correct @pixelzoom!

@kathy-phet just to clarify, on the first two screens, the Applied Force slider thumb and tweakers are already restricted to integer values (and directly set the value of F in the model). No changes need to be made to the Applied Force slider behavior, other than the number of digits to display.

kathy-phet commented 6 years ago

OK. Thanks!

pixelzoom commented 6 years ago

Note to self: The addition of 1 decimal place for Applied Force was motivated by the Energy screen, so that you'll see a change in Applied Force while using the Displacement tweakers. The change to Spring Force and the other screens was to have consistent number of decimal places for all forces, on all screens.

arouinfar commented 6 years ago

To piggyback on @pixelzoom's https://github.com/phetsims/hookes-law/issues/57#issuecomment-398831608 above. The argument for adding a decimal place to Applied Force is nicely described by @kathy-phet in https://github.com/phetsims/hookes-law/issues/57#issuecomment-397024982.

pixelzoom commented 6 years ago

These changes will only appear in master. So we'll need a new release branch and full QA cycle before republishing. Slack discussion:

Chris Malley [11:40 AM] Will any of these general Hooke’s Law changes need to be published in a maintenance release? Or can we assume that the next release will be out of master? (edited)

Amy Rouinfar [11:41 AM] I don’t think it would require a maintenance release. I think we’ll eventually do a redeploy and there will most likely need to be a full RC because it predates chipper 2.0.

Chris Malley [11:41 AM] I’m hoping the latter, since they are a bit tangled up with PhET-iO instrumentation at this point.

Amy Rouinfar [11:42 AM] I see no reason to mess with the maintenance release, then. It should definitely get a QA cycle before release.

pixelzoom commented 6 years ago

Changes requested in https://github.com/phetsims/hookes-law/issues/57#issuecomment-398827414 are in 1.1.0-dev.9, brand=phet. @arouinfar please review. If everything looks OK, we'll proceed with publishing a new PhET-iO dev version.

arouinfar commented 6 years ago

Everything looks good to me @pixelzoom. Thanks!

kathy-phet commented 6 years ago

@pixelzoom @arouinfar -

I actually am not sure. If I take the values here: x=0.232 and k=200 and calculate F, I get 46.4, but the display says 46.0. It should say 46.4.

image

pixelzoom commented 6 years ago

@kathy-phet said:

If I take the values here: x=0.232 and k=200 and calculate F, I get 46.4, but the display says 46.0. It should say 46.4.

There is an issue in SCENERY_PHET/NumberControl. By default, it constrains the value to be the same as tweaker increment, which is 1N for AppliedForce. I'll investigate.

pixelzoom commented 6 years ago

NumberControl is not the problem. Digging deeper.

arouinfar commented 6 years ago

@kathy-phet @pixelzoom when reviewing this issue, I set several different values for k and F (using their sliders) that weren't nicely divisible, and x was always properly rounded.

However, I do run into issues like the one @kathy-phet describes if I use the arm to set x, and then calculate F. Here, the model should be calculating F, which means the decimal place would often be non-zero. Sounds like this may be fixed by what @pixelzoom described in https://github.com/phetsims/hookes-law/issues/57#issuecomment-398863977.

However, there is another category of rounding issues. Here, I have set k=230 N/m and F = 50.0 N, so the model should calculate x=F/k=0.21739... which is properly rounded to 0.217. However, if I instead decide to calculate F=kx (which is not what the model is doing in this situation because I set F and k, not x and k), I end up with F=49.9 N. Even after @pixelzoom looks into the numberControl issue, there will occasionally be situations that are slightly off, depending on how users set the variables or choose to verify Hooke's Law (F=kx or x=F/k). This type of rounding issue will be unavoidable, but acceptable.

screen shot 2018-06-20 at 1 17 05 pm
pixelzoom commented 6 years ago

@arouinfar said:

... This type of rounding issue will be unavoidable, but acceptable.

Indeed, this is a symptom of the number of decimal places that you've chosen to display. There's nothing that I can do about that.

kathy-phet commented 6 years ago

Agreed. I don't see a way to avoid that scenario.

arouinfar commented 6 years ago

Indeed, this is a symptom of the number of decimal places that you've chosen to display. There's nothing that I can do about that

Agreed. I just wanted to make it clear that we should be sure to calculate the value that is derived in the model when testing.

pixelzoom commented 6 years ago

The problem reported in https://github.com/phetsims/hookes-law/issues/57#issuecomment-398856832 was due to a requirement in the model: Applied Force value must be a multiple of the tweaker increment. Our tweaker increment is still 1N, but our desired display increment is now multiples of 0.1N. No idea where this requirement came from, and I don't have time to hunt it down. So I've removed it.

@arouinfar please verify in 1.1.0-dev.10.

arouinfar commented 6 years ago

@pixelzoom things look great on the Energy screen, so everything is fine for the client.

I'm seeing some rounding issues with the Applied Force on the first two screens, most likely to do with snapping the arm to 0.001 m increments. I need to dig in a bit more, but I'll report back soon.

pixelzoom commented 6 years ago

things look great on the Energy screen, so everything is fine for the client.

Code for all 3 screens is shared, so we should get this sorted out for all 3 screens before giving the client anything.

I'm seeing some rounding issues with the Applied Force on the first two screens

Examples?

arouinfar commented 6 years ago

@pixelzoom there is some instability in the Applied Force's decimal.

Example 1

Initially, arm is dragged, and x is 0.138 m. image

k is reduced by 10, but F increases. Previously, a change in k never caused a change in F. image

k is reduced by 10 again, and F decreases. image

Example 2

Set F=50 with slider. image

Incrementally increase k with tweaker. F should remain 50, but it fluctuates.
image image image image image image image image image

pixelzoom commented 6 years ago

From your first step in Example 1:

x = F / k = 66.2 / 470 = 0.14085106382978724 = 0.141

The model doesn't (and shouldn't) know why x changed. But it sees x change and therefore computes F:

F = k x = 470 0.141 = 66.27 = 66.3

And it's the same issue in all of the other examples. This demonstrates the perils of trying to limit sig figs in the model (vs the view). And what happens when related quantities use sig figs that cause this problem.

pixelzoom commented 6 years ago

If we hadn't limited displacement to 3 decimals in the model, then we would have no change in F:

F = k x = 470 0.14085106382978724 = 66.2

But then we would have Property "loops" for some values, due to floating-point error, as we've been trying to address in https://github.com/phetsims/hookes-law/issues/52.

pixelzoom commented 6 years ago

@arouinfar and I discussed via phone and agreed that this is currently in a bad state. We couldn't possibly release something with this problem. So labeling with "block-publication".

And I have run out of time to work on this further.

arouinfar commented 6 years ago

@kathy-phet limiting x to 3 decimals in the model (as was necessary to reduce the number of messages in #52) creates devastating user-facing consequences https://github.com/phetsims/hookes-law/issues/57#issuecomment-398887691.

Because x is required to snap to the nearest 0.001 m, the applied force cannot remain constant while changing k. This is pedagogically problematic, and not acceptable for a published PhET sim.

We'll likely need to increase the number of decimal places of x in the model, and have the client filter the messages in the data stream.

pixelzoom commented 6 years ago

Unassigning until we resume work on Hooke's Law.

pixelzoom commented 6 years ago

8/16/18 design meeting:

We met about this. I'm going to undo the constraints on the model, and revisit this.

pixelzoom commented 6 years ago

In 8/16/18 design meeting, we decided that it unnecessary to constrain the number of decimal places in the model, and that it was OK to show model values (including possible floating point errors) in the PhET-iO data stream. To that end, I removed such constrains in https://github.com/phetsims/hookes-law/issues/52#issuecomment-414072168. This addresses the problems that @arouinfar reported in https://github.com/phetsims/hookes-law/issues/57#issuecomment-398887691.

In https://github.com/phetsims/hookes-law/issues/60, we revised the intervals for slider thumbs and tweakers, the number of decimal places shown for values and tick marks, etc. Since this issue was originally about that topic, and has now been addressed, I'm going to close this issue.

pixelzoom commented 6 years ago

Reopening. There are still some problems here. For example, we added a decimal place to Spring Force. Shouldn't we then add an additional decimal place to Spring Force components for the parallel system?

pixelzoom commented 6 years ago

Model value constraints have been removed, the number of decimals for displayed values has been revised, and the slider/tweaker intervals have been revised.

Here are the current values, as implemented in HookesLawConstants:

    // number of decimal places for displayed values
    APPLIED_FORCE_DECIMAL_PLACES: 1,
    SPRING_FORCE_DECIMAL_PLACES: 1,
    SERIES_SPRING_FORCE_COMPONENTS_DECIMAL_PLACES: 1, // series system
    PARALLEL_SPRING_FORCE_COMPONENTS_DECIMAL_PLACES: 2, // parallel system
    SPRING_CONSTANT_DECIMAL_PLACES: 0,
    DISPLACEMENT_DECIMAL_PLACES: 3,
    ENERGY_DECIMAL_PLACES: 1,

    // slider thumb intervals
    APPLIED_FORCE_THUMB_INTERVAL: 5, // N
    SPRING_CONSTANT_THUMB_INTERVAL: 10, // N/m
    DISPLACEMENT_THUMB_INTERVAL: 0.05, // m

    // tweaker intervals
    APPLIED_FORCE_TWEAKER_INTERVAL: 1, // N
    SPRING_CONSTANT_TWEAKER_INTERVAL: 1, // N/m
    DISPLACEMENT_TWEAKER_INTERVAL: 0.01, // m

    // drag intervals
    ROBOTIC_ARM_DISPLACEMENT_INTERVAL: 0.05, // m, Energy screen only, see #54

These changes are present in 1.1.0-dev.12. @arouinfar please review.

Here's a scenario that feels a little weird, but is not incorrect based on the number of decimal places that we've chosen. Verify that you're OK with this.

  1. Go to Intro screen.
  2. Check the "Displacement" and "Values" checkboxes.
  3. Set Spring Constant to 300 N/m.
  4. Drag robotic arm as far to the right as possible, 0.333m.
  5. Note that Applied Force is 99.9 N (300 * 0.333 = 99.9)
  6. Press the right tweaker for Applied Force, setting it to 100 N.
  7. Note that Displacement does not change. (100 / 300 = 0.33333333... = 0.333)
arouinfar commented 6 years ago

@pixelzoom the updated thumb/tweaker intervals all look good to me. Agreed that PARALLEL_SPRING_FORCE_COMPONENTS_DECIMAL_PLACES should be 2.

Here's a scenario that feels a little weird, but is not incorrect based on the number of decimal places that we've chosen. Verify that you're OK with this.

This is OK with me. It's a consequence of the precision limits, and the values are being correctly rounded. Thanks for checking!

pixelzoom commented 6 years ago

OK, we're done here. In summary, the model is unconstrained, and we revised the view.

pixelzoom commented 5 years ago

This change has not been published. So reopening and labeling with "fixed-awaiting-deploy".