CT the second order correction is larger than the first

[KatieWoe]

charges-and-fields : phet-io-fuzz : require.js : run
Query: brand=phet-io&phetioStandalone&ea&phetioValidateTandems&fuzz&memoryLimit=1000
Uncaught Error: Assertion failed: the second order correction is larger than the first
Error: Assertion failed: the second order correction is larger than the first
    at window.assertions.assertFunction (https://bayes.colorado.edu/continuous-testing/snapshot-1570010302444/assert/js/assert.js:22:13)
    at ElectricPotentialLine.getNextPositionAlongEquipotentialWithElectricPotential (https://bayes.colorado.edu/continuous-testing/snapshot-1570010302444/charges-and-fields/js/charges-and-fields/model/ElectricPotentialLine.js?:120:17)
    at ElectricPotentialLine.getEquipotentialPositionArray (https://bayes.colorado.edu/continuous-testing/snapshot-1570010302444/charges-and-fields/js/charges-and-fields/model/ElectricPotentialLine.js?:210:38)
    at ElectricPotentialLine.chargeChangedListener (https://bayes.colorado.edu/continuous-testing/snapshot-1570010302444/charges-and-fields/js/charges-and-fields/model/ElectricPotentialLine.js?:63:54)
    at TinyEmitter.emit (https://bayes.colorado.edu/continuous-testing/snapshot-1570010302444/axon/js/TinyEmitter.js?:59:55)
    at https://bayes.colorado.edu/continuous-testing/snapshot-1570010302444/axon/js/Emitter.js?:35:31
    at Emitter.execute (https://bayes.colorado.edu/continuous-testing/snapshot-1570010302444/axon/js/Action.js?:238:20)
    at Emitter.emit (https://bayes.colorado.edu/continuous-testing/snapshot-1570010302444/axon/js/Emitter.js?:60:21)
    at positionListener (https://bayes.colorado.edu/continuous-testing/snapshot-1570010302444/charges-and-fields/js/charges-and-fields/model/ChargesAndFieldsModel.js?:278:50)
    at TinyEmitter.emit (https://bayes.colorado.edu/continuous-testing/snapshot-1570010302444/axon/js/TinyEmitter.js?:59:55)
id: Bayes Chrome
Approximately 10/2/2019, 3:58:22 AM
charges-and-fields : phet-io-tests : assert
9 out of 9 tests passed. 0 failed.

Approximately 10/2/2019, 3:58:22 AM
charges-and-fields : phet-io-tests : no-assert
9 out of 9 tests passed. 0 failed.

Approximately 10/2/2019, 3:58:22 AM

[veillette]

I am responsible for this assertion. A bit of background on this assertion would be helpful so here it is.

The assertion is triggered in the process of generating the electricPotential line. The model for an electric potential line is an array of points (which will be eventually joined to form a line). Each point is generated from the previous point using a relatively simple algorithm, /*

• General Idea: Given the electric field at point position, find an intermediate point that is 90 degrees
• to the left of the electric field (if deltaDistance is positive) or to the right (if deltaDistance is negative).
• A further correction is applied since this intermediate point will not have the same electric potential
• as the targeted electric potential. To find the final point, the electric potential offset between the targeted
• and the electric potential at the intermediate point is found. By knowing the electric field at the intermediate point
• the next point should be found (approximately) at a distance epsilon equal to (Delta V)/|E| of the intermediate point. */

Mathematically, this algorithm will be exact in the limit of the deltaDistance goes to zero. In practice, the deltaDistance is finite, and every position is only approximate. The virtue of this algorithm is that one can check if the approximation is "good". If it is not then one can resort to a more precise method called getNextPositionAlongEquipotentialWithRK4( position, deltaDistance ). This second approach is more computationally intensive.

The reason for the existence of the assertions was to determine a deltaDistance that was sufficient low to be precise but at the same time have a good performance such that getNextPositionAlongEquipotentialWithRK4 would be invoke seldomly. In hindsight a console.log statements (rather than assert) would have been more appropriate.

My recommendation would be to delete the assertion in line 120 of electricPotentialLine. The line 123 takes care of the case where the second order correction is larger than the first.

Assigning to @jonathanolson

[samreid]

It seems like this error appeared just after @chrisklus @zepumph and worked on https://github.com/phetsims/charges-and-fields/issues/170. I'll apply the fix @veillette described and leave assigned to @jonathanolson for further consideration.

UPDATE: I also noticed it seems this assertion is mainly being thrown in phet-io brand.

UPDATE: It's possible that if getNextPositionAlongEquipotentialWithRK4 was previously used rarely, it may be less thoroughly tested. Perhaps it should be double checked before this issue is closed.

[samreid]

I removed the assertion, and compared values like so:

      const a = this.getNextPositionAlongEquipotentialWithRK4( position, deltaDistance );
      const b = midwayPosition.add( deltaPosition );

      console.log( a.minus( b ).getMagnitude() );

and I saw values like these:

1.0199668857868624e-7
1.0192443248814247e-7
1.042563808045265e-7
9.962076316459987e-8
1.0650432638025625e-7
9.731062282098143e-8
1.0873835732426833e-7
9.499617590556493e-8
0.0000031621631929025795
0.000003224892182420631
0.000010871578645040727
0.000010554182330117614
0.000005387183382473414
0.000005435011170029545
0.000002005537251734078

So perhaps this is OK. Still, I'll leave it open for @jonathanolson for consideration.

[jonathanolson]

Removing the assertion sounds good. The Runge-Kutta looks nicely done (using the perpendicular normal vector to the field as the derivative). Avoiding trigonometry for the perpendicular computation might help performance (e.g. -y, x in this case).

[samreid]

There are 5 occurrences of normalize().rotate( Math.PI / 2 ), I'm also seeing many other opportunities for performance improvements by eliminating allocations. I'll create a new issue for these and close this one.