Stop Expand losing a term that takes no power (#851) (#852)

Expand turned RR + 1 into NaN, and with it ZZ + 1, CC + 1, QQ + 1, BB + 1 and
true + 1. A 2.0 regression: 1.4.0 leaves all of them alone. Expand is public,
so this was not only a Simplify artefact.

CollectLikeTerms reduces every factor to a base and an exponent and then puts
the term back together. A factor that appeared once came back as base^1, and
raising to the first power is an identity only where the power is defined at
all -- RR^1, ZZ^1 and true^1 are NaN. A factor that appeared once now goes
back as itself, and a second guard discards any collection that introduced
NaN, which covers the same fault above the first power.

Sets that can be shifted still are: { 1, 2 } + 1 is { 2, 3 }.

Found by running CSharpMath's suite against 2.0.0-preview.1 -- 955 green on
1.4.0, two red on the preview. Rebuilt against this change it is one.

Author: Rafael-SOWNet

Sources/AngouriMath/Functions/Evaluation/Evaluation.Definition.cs

@@ -290,7 +290,17 @@ private static Entity CollectLikeTerms(Entity expanded)
                     coefficients[monomialKey] = coefficient;
                     Entity monomial = 1;
                     foreach (var key in exponents.Keys.OrderBy(k => k, System.StringComparer.Ordinal))
-                        monomial = (monomial * bases[key].Pow(exponents[key])).InnerSimplified;
+                    {
+                        // A factor that appeared once goes back as itself, not as base^1.
+                        // Raising to the first power is an identity only where the power is
+                        // defined at all: RR^1, ZZ^1 and true^1 are all NaN, so writing the
+                        // factor that way turns an expression that had a value into one that
+                        // does not. https://github.com/asc-community/AngouriMath/issues/851
+                        var factor = exponents[key] == Integer.Create(1)
+                            ? bases[key]
+                            : bases[key].Pow(exponents[key]);
+                        monomial = (monomial * factor).InnerSimplified;
+                    }
                     monomials[monomialKey] = monomial;
                 }
             }
@@ -306,7 +316,18 @@ private static Entity CollectLikeTerms(Entity expanded)
                 var term = (coefficients[key] * monomials[key]).InnerSimplified;
                 result = result == Integer.Create(0) ? term : result + term;
             }
-            return result.InnerSimplified;
+            var collected = result.InnerSimplified;
+
+            // Collecting is an improvement, never a requirement, so a collection that
+            // introduced NaN is discarded rather than returned. The decomposition into
+            // coefficient and monomial only describes a node that multiplies and takes
+            // powers; a factor of any other kind -- a set raised above the first power,
+            // say -- reassembles into something undefined. Keeping the expanded form is
+            // always sound. https://github.com/asc-community/AngouriMath/issues/851
+            if (collected.Nodes.Any(node => node == MathS.NaN)
+                && !expanded.Nodes.Any(node => node == MathS.NaN))
+                return expanded;
+            return collected;
         }
 
         /// <summary>

Sources/Tests/UnitTests/Common/ExpandCollectsTermsTest.cs

@@ -69,5 +69,34 @@ public void ExpansionPreservesValue(string input)
         public void CancellingTermsKeepTheirDomainCondition() =>
             Assert.Equal("0 provided not x = 0".ToEntity(),
                 "(4a - 2) / (2x) + (1 - 2a) / x".ToEntity().Simplify());
+
+        // Collecting reduces each factor to a base and an exponent and then puts it back
+        // together. Writing a factor that appeared once as base^1 is an identity only where
+        // the power is defined at all -- RR^1 and true^1 are NaN -- so a term of a kind that
+        // does not take powers used to come back undefined.
+        // https://github.com/asc-community/AngouriMath/issues/851
+        [Theory]
+        [InlineData("RR + 1")]
+        [InlineData("1 + RR")]
+        [InlineData("RR + x")]
+        [InlineData("ZZ + 1")]
+        [InlineData("QQ + 1")]
+        [InlineData("CC + 1")]
+        [InlineData("BB + 1")]
+        [InlineData("true + 1")]
+        [InlineData("RR * RR + 1")]
+        public void ATermThatTakesNoPowerIsNotLost(string input)
+        {
+            Assert.NotEqual(MathS.NaN, input.ToEntity().Expand());
+            Assert.NotEqual(MathS.NaN, input.ToEntity().Simplify());
+        }
+
+        // The sets that can be shifted elementwise still are -- the guard above must not
+        // turn collecting off for them.
+        [Theory]
+        [InlineData("{ 1, 2 } + 1", "{ 2, 3 }")]
+        [InlineData("[0; 1] + 1", "[1; 2]")]
+        public void ASetThatCanBeShiftedStillIs(string input, string expected) =>
+            Assert.Equal(expected.ToEntity(), input.ToEntity().Simplify());
     }
 }