CT: adjacent divide should have been collapsed

[pixelzoom]

This happened for the first time on 1/18/2021, 8:45:48 PM. It's either a new issue introduced by common-code changes, or (more likely) something that is unlikely to be exercised by CT. Note that a specific equation ([[x]/3]/1) is shown in the assertion message, so I would start by trying to manually reproduce that equation.

function-builder : fuzz : unbuilt
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611027948396/function-builder/function-builder_en.html?continuousTest=%7B%22test%22%3A%5B%22function-builder%22%2C%22fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1611027948396%22%2C%22timestamp%22%3A1611032425249%7D&brand=phet&ea&fuzz&memoryLimit=1000
Query: brand=phet&ea&fuzz&memoryLimit=1000
Uncaught Error: Assertion failed: adjacent divide should have been collapsed: [[x]/3]/1
Error: Assertion failed: adjacent divide should have been collapsed: [[x]/3]/1
at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611027948396/assert/js/assert.js:25:13)
at new HelpfulEquationNode (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611027948396/function-builder/js/common/view/equations/HelpfulEquationNode.js:227:21)
at EquationPanel.updateEquations (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611027948396/function-builder/js/common/view/equations/EquationPanel.js:141:32)
at https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611027948396/function-builder/js/common/view/equations/EquationPanel.js:110:14
at TinyEmitter.emit (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611027948396/axon/js/TinyEmitter.js:71:18)
at https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611027948396/axon/js/Emitter.js:35:29
at Emitter.execute (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611027948396/axon/js/Action.js:225:18)
at Emitter.emit (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611027948396/axon/js/Emitter.js:60:19)
at MathBuilder.addFunctionInstance (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611027948396/function-builder/js/common/model/builder/Builder.js:149:33)
at MathBuilder.addFunctionInstance (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611027948396/function-builder/js/common/model/builder/MathBuilder.js:35:11)
id: Bayes Chrome
Snapshot from 1/18/2021, 8:45:48 PM

[pixelzoom]

This is still occurring very occassionally as of 8/2/2021.

[pixelzoom]

I haven't seen this in a long while. It's not blocking, and will not cause problems in practice.

[pixelzoom]

Based on the error message: adjacent divide should have been collapsed: [[x]/3]/1 ...

The failing assertion is in HelpfulEquationNode.ts, the Node that is shown in the Equation screen, with the bottom (equation) drawer open, and "simplify" checkbox unchecked.

assert && assert( !previousOperator || previousOperator !== FBSymbols.DIVIDE,
  `adjacent divide should have been collapsed: ${equation.toString()}` );

To reproduce:

• go to the Equations screen
• open the bottom (Equation) drawer
• set up the builder configuration / 3 + 1 / 1, as shown below
• attempt to change the middle function to + 0, and the assertion will fail

In practice (assertions disabled) the screenshot below is what is shown.

So what should be shown? The ground truth for the "helpful" format is document in equation-formats.md.

Based on this rule:

(1) Adjacent addition and subtraction operations are collapsed, e.g.: + 3 - 2 → x + 1. If they collapse to zero, then the operations are dropped entirely, e.g.: * 2 + 3 - 3 → 2x

The + 0 in / 3 + 0 / 1 should be dropped, resulting in / 3 / 1. Then based on this rule:

(6) Adjacent division operations are collapsed, e.g.: / 3 / 2 → x/6

... / 3 / 1 should be collapsed to / 3.

So what is displayed should be y = x / 3.

[pixelzoom]

The fix for this needs to be applied in the constructor of HelpfulEquation.js, where we're currently doing nothing for the if ( currentOperand === 0 ) case.

      if ( currentOperator === FBSymbols.PLUS || currentOperator === FBSymbols.MINUS ) {

        if ( currentOperand === 0 ) {
          // ignore plus or minus zero
        }

[pixelzoom]

In HelpfulEquation.js, lne 113 is likely the buggy line:

      if ( stack.length > 0 ) {
        previousFunction = stack[ stack.length - 1 ];
113     previousOperator = currentOperator;
        previousOperand = previousFunction.operandProperty.get();
      }

It should have been:

113     previousOperator = previousFunction.operator;

[pixelzoom]

Fixed in the above commit. Screenshot:

[pixelzoom]

To verify in 1.2.0-rc.1, you'll need to compare its behavior to 1.0.25 (the published version).

• You'll be looking at the unsimplified (aka "helpful") equation format, shown in the Equations screen, with the bottom "drawer" open and the "simplified" checkbox unchecked. (see screenshot in https://github.com/phetsims/function-builder/issues/135#issuecomment-1137967445.
• Set up various combinations of functions in the builder, and verify that they look the same in 1.2.0-rc.1 and 1.0.25.
• When the middle function is '+0' or '-0', then 1.2.0-rc.1 and the published version should be different. In that case, the '+0' and '-0' should be ignored. And (if appropriate) functions on either side of '+0' and '-0' should be combined. For example, '/ 3 +0 /1' will be y = x/3 in 1.2.0-rc.1, but y = x / 3 / 1 in 1.0.25.

If you have questions about whether the format is correct, see equation-formats.md or ask me.

[Nancy-Salpepi]

In my testing, the unsimplified equations look the same in 1.2.0-rc.1 and 1.2.25 except in the instances mentioned in the above https://github.com/phetsims/function-builder/issues/135#issuecomment-1139067805. The unsimplified equations are following the format outlined in https://github.com/phetsims/function-builder/blob/master/doc/equation-formats.md.

Closing but will reopen if something comes up.