ConcentrationDescriber `*ToIndex` functions should be implemented with a loop or `LinearFunction`.

[zepumph]

Unlike other places where the regions are mapped non-linearly, these regions (save the edge single value regions) are linearly spaced. Instead of using the duplicative format for other cases, (the one I recommended using in situations like soluteAmountToIndex), let's use a for loop or a LinearFunction.

I think a loop would be the simplest, but also you could look into creating a LinearFunction for each new maxConcentration, and then using that to calculate the middle regions. And for extra credit, if you were worried about memory, you could keep a map of all the linear functions created so far based on the maxConcentration, since there are only N of those (where n is the number of solutes).

from https://github.com/phetsims/molarity/issues/171

Let me know if this doesn't make sense.

[twant]

@zepumph just pushed changes, although may want to hold off on reviewing/closing this one until I take a look at the other describers!

[zepumph]

Above I committed a potential simplification to the solids loop. I wasn't understanding why we were subtracting values, and I also felt like it was strange to go from zero to the length. I tested and found that it was working as I was expecting it to, with even ranges of solids for Potassium permanganate from saturated to min vol max solute amount.

For the concentrationToIndex I was a bit more confused, enough so that I didn't want to commit anything before discussing.

We even regions save two single value regions (for min and max). I wasn't sure why there was so much added complexity, can't we just test those two regions, and then otherwise treat it the same as solids amount? Why do we need to subtract a hard coded value?

How does this look to you?

Index: js/molarity/view/describers/ConcentrationDescriber.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- js/molarity/view/describers/ConcentrationDescriber.js   (revision bc98c2794df2b26ea464c4666458911bdcab80ca)
+++ js/molarity/view/describers/ConcentrationDescriber.js   (date 1575929413934)
@@ -466,14 +466,20 @@
     // Concentration regions are evenly spaced within the region from 0 to max concentration for a given solute except
     // for the lowest region (zero) and the highest region (max concentration) which are single value regions.
     const scaleIncrement = maxConcentration / ( CONCENTRATION_STRINGS.length - 2 );
-    const concentrationRounded = Util.toFixed( concentration, MolarityConstants.CONCENTRATION_DECIMAL_PLACES );

-    if ( concentrationRounded > maxConcentration - .001 ) {
+    // compare against unrounded concentration since these two are single value regions
+    if ( concentration === maxConcentration ) {
       return CONCENTRATION_STRINGS.length - 1;
     }
+    else if ( concentration === 0 ) {
+      return 0;
+    }
     else {
-      for ( let i = 0; i < CONCENTRATION_STRINGS.length - 1; i++ ) {
-        if ( concentrationRounded <= scaleIncrement * i - .001 ) {
+      const concentrationRounded = Util.toFixed( concentration, MolarityConstants.CONCENTRATION_DECIMAL_PLACES );
+
+      // don't count the first and last concentration string
+      for ( let i = 1; i < CONCENTRATION_STRINGS.length - 2; i++ ) {
+        if ( concentrationRounded <= ( i + 1 ) * scaleIncrement ) {
           return i;
         }
       }

[zepumph]

@twant and I worked on this, I'll give a spot check before closing.

[zepumph]

All looks good. Closing