Hitting max (and min?) integer limit

[phet-steele]

On the Solving screen, the user can continuously multiply and increase the size of the number on the scale. Once hitting the memory limit, the sim just freezes (without ?ea). This can be reached with multiplication and division, both positive and negative:

@ariel-phet suggested that the sim should still allow users to interact in this case, but operations that go beyond this limit just fail to occur. @pixelzoom did you also mention that we shouldn't see floating point numbers (because there they are)?

Seen on macOS 10.13.3 Chrome, master 3/15 noon.

[pixelzoom]

We'll need to decide what happens when we reach the limits of numeric representation.

We also need to stop before exponential notation is used.

[amanda-phet]

So this seems like the scenario I was describing before. If you try to divide by 10 to "undo" all of your previous operations, you can't get back to 1 because of the rounding.

Anyway, I think we should have a max number that an integer/numerator/denominator can reach. At that time, we could either

(1) disable the "apply" button if an operation would make the maxed out number any larger, and/or (2) show a dialog (similar to Trig Tour and Plinko) that communicates to the user that they've reached a point they can't go past. Obviously something cute with PhET girl.

[pixelzoom]

Which option(s) would you like me to proceed with?

[pixelzoom]

@amanda-phet Shall we pick a "reasonable" maximum value for the numerator and denominator, say 1,000,000? That would ensure that the terms on the scale don't get insanely small - see screenshot below. Or is there pedagogical value to having larger values?

[pixelzoom]

If there are more than 21 digits before the decimal point, JavaScript switches to displaying numbers in exponential format. See http://2ality.com/2012/03/displaying-numbers.html

[pixelzoom]

In a Slack conversation with @phet-steele, he pointed out that this issue is not specific to the universal operation. The limit can also be exceeded by dragging terms from the toolboxes to the scale. And that is not addressed by the options proposed in https://github.com/phetsims/equality-explorer/issues/48#issuecomment-373513075.

[pixelzoom]

Raising priority to high since I'm currently revising the universal operation, and this is related.

[amanda-phet]

@amanda-phet Shall we pick a "reasonable" maximum value for the numerator and denominator, say 1,000,000? That would ensure that the terms on the scale don't get insanely small - see screenshot below. Or is there pedagogical value to having larger values?

We decided long ago that there wasn't harm in going into the realm of unreadable, because there's no mathematical reason why you couldn't keep applying operations. So, I don't think we need to stop that soon (you could get there in just 6 clicks of x10).

The limit can also be exceeded by dragging terms from the toolboxes to the scale.

This makes the solution a bit more difficult. @phet-steele any ideas? This makes me think we should go with option (2) where the balance maxes out and you get a dialog communicating that you've reached a limit, but I'm not sure what the sim should do once you close the dialog. Should we force the user to clear the balance in the dialog? It's not like they were doing anything productive by the time they reach our limit.

[pixelzoom]

Re the proposal in https://github.com/phetsims/equality-explorer/issues/48#issuecomment-373553462, @amanda-phet said:

We decided long ago that there wasn't harm in going into the realm of unreadable, because there's no mathematical reason why you couldn't keep applying operations. So, I don't think we need to stop that soon (you could get there in just 6 clicks of x10).

The proposal was to pick a min/max. The value I threw out (1,000,000) was less important.

We are limited to at most 1E20 (20 clicks of x10, see https://github.com/phetsims/equality-explorer/issues/48#issuecomment-373554866), so it is highly likely that this limit will be encountered by the student. There is certainly no UX value to going beyond 1,000,000. So if there's no pedagogical value, I think it would be preferable to choose min/max that keeps the UI legible.

If the general proposal sounds OK, then please specify what min/max is acceptable.

[pixelzoom]

For the scenario where dragging something to scale would cause the limit to be exceeded, how about: as the term is being put on the scale (either by dragging or animating), display the dialog, then delete all terms that are currently animating or being dragged. (similar to changing the lock control)

[pixelzoom]

Raising priority to "top", since this has gotten complicated. We absolutely must have a proposed solution for this by early this coming week.

[amanda-phet]

Playing with the sim a bunch and noticed rounding after 1•10^16 (e.g., when I divided 100000000000000000 ÷ 9 I should see 100000000000000000/9 but instead saw 111111111111111112). So, I propose that we make 1•10^16 the largest integer (either in the numerator or denominator). The values in the equation are legible enough to me at this point, and the user won't reach it really soon if they are playing around with maxing out the sim.

For the scenario where dragging something to scale would cause the limit to be exceeded, how about: as the term is being put on the scale (either by dragging or animating), display the dialog, then delete all terms that are currently animating or being dragged. (similar to changing the lock control)

I like this idea a lot. If using the operator would cause the limit to be exceeded, we would display the dialog before the operation ever gets applied.

@pixelzoom let me know if you'd like to chat more about this on zoom.

[pixelzoom]

I'm still trying to understand... What is the motivation for numbers as large 1E16 ? Regardless of whether a number that large is legible in the equation, I find it hard to believe that anyone can (or will) count digits beyond perhaps 10. Is a maximum this large solely motivated by the hope that users won't encounter the limit?

[amanda-phet]

Yes, I'd like the limit to be difficult to encounter. But I'm not too invested in any particular limit, so I'd be fine with something smaller. How about 1E9?

[pixelzoom]

The current proposal for the universal operation is to check whether the maximum would be exceeded when the "go" button is pressed. That proposal has 2 problems: (1) It requires computing what the result would be before applying the operation, which means additional/redundant computations. (2) It doesn't fully protect us from exceeding the maximum. Because there's an animation involved, the contents of the scale may change between the time that the user presses the "go" button and the time that the operation is applied to the scale.

Here's another proposal:

For any operation, whether applied via the universal operation or by dragging terms to the scale, check whether the maximum will be exceeded just as the term is about to be put on the scale. If the maximum would be exceeded, display the dialog, and delete all terms that are currently animating or being dragged (i.e. all terms that are not on the scale). Behavior is the same for all scenarios, and is similar to how we treat changes to the lock control.

@amanda-phet Does this sound acceptable?

[pixelzoom]

@amanda-phet Please mock up the dialog. For the dialog message, keep in mind that there are 3 distinct scenarios: constant would exceed max; fraction numerator would exceed max; fraction denominator would exceed max. Ideally a single message (and therefore single translated string) that covers all cases, or one message for each of the 3 scenarios.

[amanda-phet]

@amanda-phet Does this sound acceptable?

Yes that solution makes sense to me. If something like "+2" is being animated to the scale, the animation would complete but wouldn't actually be applied- is that correct? I imagine the dialog would also display quickly enough that we probably wouldn't notice that nothing changed as a result of adding 2.

[pixelzoom]

.... wouldn't actually be applied- is that correct?

Yes, that's correct.

[amanda-phet]

Would the dialog appear every time the user clicks the "go" button on an invalid operation? Or would we just show the dialog the first time, and after that the operation just doesn't get applied?

I am thinking we would show the dialog every time, but that seems like it could be annoying.

[pixelzoom]

Would the dialog appear every time the user clicks the "go" button on an invalid operation? Or would we just show the dialog the first time, and after that the operation just doesn't get applied?

Showing the dialog when the "go" button is clicked is not part of the current proposal. The proposal is to show the dialog as a term is about to be put on the scale. For universal operations, this is after the "go" button has been pressed and the operation has been animated towards the plates.

I believe we need to show the dialog every time, since there are multiple scenarios (constant too big; numerator too big; denominator too big). That might not be obvious, and (unless the dialog message is very specific) it might take the student several times of experiencing the dialog to determine what the limit is.

[amanda-phet]

This is the latest draft:

The wording is definitely tricky because I want to only have one dialog for all three scenarios that would prompt it:

constant would exceed max; fraction numerator would exceed max; fraction denominator would exceed max. Ideally a single message (and therefore single translated string) that covers all cases, or one message for each of the 3 scenarios.

[pixelzoom]

Re the "latest draft" above...

"That value is too large" is not appropriate because the operation that would result in "that value" is not going to be applied, and "that value" is therefore not displayed. The message that needs to be communicated (in some user-friendly language) is: Your attempt to put something on the scale has been canceled, because it would cause constant/numerator/denominator to exceed the maximum value that we can represent.

We probably also don't want to use "sim".

[pixelzoom]

Slack discussion with @amanda-phet.

We're on the same page wrt https://github.com/phetsims/equality-explorer/issues/48#issuecomment-375800593.

To prevent users from pressing the "go" button in a rapid-fire manner (which serves no pedagogical purpose), I suggested that we disable the "go" button while the operation is animating. This will make it less likely for that the limit will be encountered. All other UI elements will remain enabled. @amanda-phet gave this suggesting the green light.

[pixelzoom]

In 1.0.0-dev.79, the 'go' button is now disabled until the animation completes. Query parameter ?goButtonEnabled keeps the 'go' button enabled at all times, allow you to press it "rapid fire" style, for the purposes of testing limits.

[pixelzoom]

I did a quick evaluation of what it's going to take to implement the proposal in https://github.com/phetsims/equality-explorer/issues/48#issuecomment-375443211. It's going to be a big project, probably requiring yet-another round of expensive architectural changes.

Detailed notes to self: There is currently nothing that keeps a running total of what's on the scale, so nothing to consult about what the current "subtotal" is for a term, and what adding an additional term might do to that total. To make matters worse, positive and negative term creators (e.g. 'x' and '-x') have no knowledge of each other, and add things to the scale independently. So it's currently not possible to get (for example) the total x coefficient for all terms on the scale. The equation does have this information, but it sums things up after they've been added to the scale. And term creators (who are responsible for adding terms to the scale) have no knowledge of the equation.

[pixelzoom]

More notes to self: Consider having a "term accumulator" associated with each positive/negative term creator pair, which keeps a running total of the constant value (for constant terms) or coefficient (for variable terms). Consult the term accumulator to decide if an operation would exceed the limits. This could also simplify EquationNode, since it could use term accumulators instead of term creators, and therefore not have to sum terms.

[pixelzoom]

I'm going to proceed with this using a placeholder dialog, and assume that there will be one dialog, with one message. @amanda-phet if that's not a good assumption, please tell me asap.

[pixelzoom]

I pursued the "term accumulator" idea (https://github.com/phetsims/equality-explorer/issues/48#issuecomment-375982682) today, and it turned into a big mess, which I ultimately reverted :( Back to the drawing board...

[pixelzoom]

More notes to self: Now I'm (re)considering the merging of positive and negative term creators into a single term creator (e.g. "x term creator"), with separate TermCreatorNodes for positive and negative terms (e.g. 'x terms' and '-x terms' ). This is something I had previously investigated and abandoned in branch 'combined-term-creators', see https://github.com/phetsims/equality-explorer/issues/44. I deleted that branch, but it wouldn't be relevant at this point anyway.

[pixelzoom]

In the preceding commit, I made the major architectural change of combining positive and negative pairs of term creators. So now there's one "term creator" per term type, and each term creator can be consulted about the total constant or coefficient values for terms on a plate.

Bonus: this also simplified EquationNode, see https://github.com/phetsims/equality-explorer/commit/7e8b1271e720d2ba62a966018024e3474505bc3a.

Anti-bonus: Query parameters for populating the scale are no longer supported.

[pixelzoom]

But this change totally broke the "organize" feature. Ugh.

[pixelzoom]

Organize feature is fixed. Now I'll see what it takes to restore the query parameters that populate the scale.

[pixelzoom]

3/29/18 design meeting:

Consensus is to have 1 dialog message that covers all situations.

Message brainstorming yielded: "Oops! That will make a number that is too big for the sim. Try a different operation."

Dialog should have a close button, for a11y.

[amanda-phet]

[pixelzoom]

@amanda-phet please send me the artwork (.ai file) and credits for the PhET girl shown in the above dialog mockup.

[pixelzoom]

Stuff that was added in the above commit:

• query parameter largestInteger, default 1E9. Use ?largestInteger=100 for testing. • TermCreator.isNumberLimitExceeded for detecting if a term would exceed the limit • TermCreator.numberLimitExceededEmitter for notifying listeners when the limit is exceeded • Scene.disposeTermsNoOnScale that is called when the limit is exceeded • OperationCanceledDialog that is displayed when the limit is exceeded • detect when the limit is exceeded for terms that are dragged (TermDragListener)

Next up... Add detection for universal operations.

[pixelzoom]

Notes to self about detecting the limit for universal operations... Detection has to be done at the Scene level, since if any term would exceed the limit as the result of the operation, then no changes should be made to what's on the scale. The easiest thing to do might be to (internally) use the snapshot feature to capture what's on the scale. Then start trying to apply the operation to one TermCreator at a time. If the limit is encountered, bail and restore the snapshot. This probably needs to be done in OperationsScene.applyOperation.

[pixelzoom]

More notes to self... Detecting that the limit is exceeded currently results in deleting of all terms that are not on the scale. But it should also stop any universal operation animations that are in progress.

[pixelzoom]

Reopening. Accidentally closed by commit message.

To implement this for universal operations, I implemented the ideas in https://github.com/phetsims/equality-explorer/issues/48#issuecomment-377435219. That is, if applying the operation resulted in any term exceeding the limit, I restored a snapshot that was taken before the operation was applied.

[pixelzoom]

This feature is completed. I emailed a dev version to the team, and @kathy-phet reported a bug. That bug was fixed in 1.0.0-dev.85.

From that email:

The limit defaults to 1E9. For testing, you can change the limit via a query parameter, for example: ?largestInteger=10

The limit can be exceeded when dragging a term to the scale, or when applying a universal operation. If these actions would result in a term on the scale whose numerator or denominator (absolute value) exceeds the limit, then what’s on the scale is not modified, and 4 things happen: (1) the action is canceled (2) universal operation animations that are in progress are canceled (3) any terms that are not on the scale are deleted (4) the “Oops!” dialog is displayed

In your testing, please provide feedback on:

• How often did you hit the limit? We’d like this to be encountered rarely. • When you hit the limit, does the behavior match the design? • Does 1E9 seem like an appropriate limit? We can technically go as high as 1E20. • Any bugs encountered?

@ariel-phet's feedback was:

I find it a bit odd that with nothing on the scale you can apply an operation like "divided by 4" and it just goes into the ether. But other than that, seems to look good.

I replied:

The alternative would be to disable the ‘go’ button when there’s nothing on the scale and the operation is ’times’ or ‘divide’. That may be more confusing, and doesn’t facilitate a discussion about it.

@kathy-phet replied:

I like the current behavior better than that disable option.

[pixelzoom]

@amanda-phet Let me know your thoughts on 1.0.0-dev.85, and whether you'd like any changes.

@phet-steele Since you reported this, I'd also like your feedback on our solution in 1.0.0-dev.85. See https://github.com/phetsims/equality-explorer/issues/48#issuecomment-377658699 for a summary of what the solution is, and what feedback is desired. Thanks!

[amanda-phet]

I agree with @kathy-phet that the button should not be disabled. 0 ÷ 4 is 0, so it makes perfect sense to me. I like that the button is disabled until the operation is complete. It forces me to think about what I’m doing, rather than mindlessly clicking enter, and the timing of the dialog makes the most sense this way. I don’t have anything else to add!

[pixelzoom]

Looks like we're done here. Closing.

[pixelzoom]

Oops. Reopening for @phet-steele feedback, since he created this issue.

[phet-steele]

I agree with https://github.com/phetsims/equality-explorer/issues/48#issuecomment-378332835.

How often did you hit the limit? We’d like this to be encountered rarely.

That's a biased question. I was purposefully trying to reach the limit, so my answer is "VERY often".

When you hit the limit, does the behavior match the design?

Sure do.

Does 1E9 seem like an appropriate limit? We can technically go as high as 1E20.

1E9 is fine, but so is 1E20. I wouldn't mind either.

Any bugs encountered?

Yea, they were #58 and #59, both fixed by some mystical wizard. Then there is a new one, #61.

[pixelzoom]

Thanks @phet-steele. Looks like any remaining (related) issues are being tracked elsewhere, so closing.

[pixelzoom]

I've deleted the goButtonEnabled query parameter. You can accomplish the same thing (getting to the number limit fast) by using largestInteger query parameter.