Count a division by what it is, not by how it was written (#950)

BREAKING-CHANGES.md

@@ -23,6 +23,36 @@ read first.
 |---|---|---|---|
 | | `(Entity.Number)someBigInteger` | `FormatException: Illegal character found`, for almost every value | the number |
 | **Silent** | `"1/x".Integrate("x")`, and every antiderivative with a logarithm | `ln(abs(x)) + C` | `ln(x) + C` |
+| **Silent** | `CostModel.FewestDivisions.Cost(y ^ (1 * (-1)) * x)` | `0.007`, cheaper than `x / y` | `1.007` |
+
+### `FewestDivisions` and `FewestRadicals` score by value, not by spelling
+
+Both tested the exponent's **node**: `Powf(_, Real { IsNegative: true })` and
+`Powf(_, Rational and not Integer)`. In `y ^ (1 * (-1))` the exponent is a `Mulf`, so neither
+fired, and the writing whose division the model could not see scored *cheapest* — under the
+criterion whose whole job is to remove divisions.
+
+| expression | `FewestDivisions` was | is |
+|---|--:|--:|
+| `x / y` | 1.003 | 1.003 |
+| `y ^ (-1) * x` | 1.005 | 1.005 |
+| `y ^ (1 * (-1)) * x` | **0.007** | **1.007** |
+
+All three are one value written three ways, so one division each is the answer, and the written
+form — being the smallest tree — is now correctly the cheapest. `FewestRadicals` had the same shape
+of test and so the same blind spot: `x ^ (2 ^ (-1))` is a square root and is now counted as one.
+
+**`Simplify`'s output does not change.** Measured on a build of each, twelve division-heavy inputs
+under both models, twenty-four results, byte-identical. `Simplify` scores candidates it has already
+evaluated, so it did not reach the gap; what reaches it is a caller scoring expressions it did not
+build — extraction on an e-graph, where every writing of a value is a member of one class at once,
+which is how this was found. Only `Cost` returns different numbers, and only for an expression with
+an unevaluated exponent.
+
+`Default` and `SmallestTree` are deliberately **unchanged**. They count how an expression is
+written, and `y ^ (1 * (-1)) * x` really is a bigger tree — for them the node as written is not an
+approximation of the question, it is the question.
+[#950](https://github.com/asc-community/AngouriMath/issues/950).
 
 ### An antiderivative with a logarithm drops the absolute value unless the codomain is real
 

Sources/AngouriMath/Core/CostModel.cs

@@ -1,4 +1,4 @@
-//
+//
 // Copyright (c) 2019-2026 Angouri.
 // AngouriMath is licensed under MIT.
 // Details: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md.
@@ -27,6 +27,27 @@ namespace AngouriMath.Core
     /// smallest tree, fewest radicals — that <see cref="All"/> now holds.
     /// </para>
     /// <para>
+    /// <b>A model counts one of two things, and which one decides what it may read.</b>
+    /// <see cref="SmallestTree"/> and <see cref="Default"/> count <i>how the expression is
+    /// written</i>: <c>y ^ (1 * (-1)) * x</c> really is a bigger tree than <c>x / y</c>, so
+    /// reading the node as written is not an approximation, it is the question. The others count a
+    /// <i>mathematical feature</i> — is there a division here, is there a radical — which is a
+    /// property of the value and not of the spelling, so they ask what an exponent
+    /// <see cref="Entity.Evaled"/> to rather than what it was typed as.
+    /// </para>
+    /// <para>
+    /// The distinction is worth stating because getting it wrong is silent and backwards:
+    /// <see cref="FewestDivisions"/> used to test the exponent's node, so
+    /// <c>y ^ (1 * (-1)) * x</c> — a division written where the test could not see it — scored
+    /// <i>cheaper</i> than <c>x / y</c>, under the one criterion whose whole job is to remove
+    /// divisions. That is
+    /// <a href="https://github.com/asc-community/AngouriMath/issues/950">#950</a>. <b>A caller
+    /// writing their own model wants to decide which of the two kinds it is</b>, because nothing
+    /// here can decide it for them: an expression handed to a cost model has not necessarily been
+    /// evaluated, and on an e-graph — where every writing of a value is a member of one class at
+    /// once — it certainly has not.
+    /// </para>
+    /// <para>
     /// <b>Every model here counts nodes a little, even the ones that are about something else.</b>
     /// A criterion that counts only its own feature ties constantly, and a tie is settled by
     /// whichever candidate the search happened to generate first — which is an accident rather
@@ -76,24 +97,29 @@ public sealed record CostModel(string Name, string Description, Func<Entity, dou
         /// <summary>Prefers the expression with fewest divisions, then fewest nodes.</summary>
         /// <remarks>
         /// A negative power is a division written differently, so it counts too — otherwise the
-        /// model would merely move divisions rather than remove them.
+        /// model would merely move divisions rather than remove them. For the same reason the
+        /// exponent is read by value: <c>y ^ (1 * (-1))</c> is a division however it is spelled,
+        /// and a test that missed it would rate the unevaluated writing cheapest.
         /// </remarks>
         public static CostModel FewestDivisions { get; } = new(
             nameof(FewestDivisions),
             "Fewest divisions, counting a negative power as one, then fewest nodes.",
             static expr => Feature(expr, static node =>
-                node is Divf || node is Powf(_, Real { IsNegative: true })));
+                node is Divf
+                || node is Powf(_, var exponent) && exponent.Evaled is Real { IsNegative: true }));
 
         /// <summary>Prefers the expression with fewest radicals, then fewest nodes.</summary>
         /// <remarks>
         /// A radical is a power by a non-integer rational — <c>sqrt(x)</c> is <c>x ^ (1/2)</c>
-        /// here, and there is no separate root node to count.
+        /// here, and there is no separate root node to count. The exponent is read by value, so
+        /// <c>x ^ (2 ^ (-1))</c> counts as the radical it is.
         /// </remarks>
         public static CostModel FewestRadicals { get; } = new(
             nameof(FewestRadicals),
             "Fewest fractional powers, then fewest nodes.",
             static expr => Feature(expr, static node =>
-                node is Powf(_, Rational and not Integer)));
+                node is Powf(_, var exponent)
+                && exponent.Evaled is Rational and not Integer));
 
         /// <summary>
         /// Every model here, so a caller can offer the choice rather than hard-code one.

Sources/Tests/UnitTests/Core/CostModelTest.cs

@@ -1,4 +1,4 @@
-//
+//
 // Copyright (c) 2019-2026 Angouri.
 // AngouriMath is licensed under MIT.
 // Details: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md.
@@ -133,6 +133,58 @@ public void AFeatureModelStillBreaksTiesBySize()
             Assert.True(CostModel.FewestDivisions.Cost(small) < CostModel.FewestDivisions.Cost(large));
         }
 
+        /// <summary>
+        /// https://github.com/asc-community/AngouriMath/issues/950 -- the three are one value
+        /// written three ways, and a model that counts divisions has to count the same number in
+        /// each. It used to read the exponent's node, so the writing whose division it could not
+        /// see scored cheapest, under the criterion whose whole job is removing them.
+        /// </summary>
+        [Fact]
+        public void AFeatureModelCountsTheValueAndNotTheSpelling()
+        {
+            var written = "x / y".ToEntity();
+            var negativePower = MathS.Pow("y".ToEntity(), -1) * "x".ToEntity();
+            var unevaluatedExponent = MathS.Pow("y".ToEntity(), "1".ToEntity() * (-1)) * "x".ToEntity();
+
+            // The middle one is the point: its exponent is a Mulf, not a negative Real.
+            Assert.IsNotType<Entity.Number>(
+                Assert.IsType<Entity.Powf>(unevaluatedExponent.DirectChildren[0]).Exponent);
+
+            // One division each, so the feature term is equal and only the node term separates
+            // them -- which means the written form, being smallest, must come out cheapest.
+            foreach (var e in new[] { written, negativePower, unevaluatedExponent })
+                Assert.True(CostModel.FewestDivisions.Cost(e) >= 1,
+                    $"{e.Stringize()} is a division and must be counted as one");
+            Assert.True(CostModel.FewestDivisions.Cost(written)
+                        < CostModel.FewestDivisions.Cost(unevaluatedExponent),
+                "the writing whose division is hardest to see must not be the cheapest");
+        }
+
+        /// <summary>The same blind spot, in the other feature model.</summary>
+        [Fact]
+        public void FewestRadicalsSeesARadicalWhoseExponentIsUnevaluated()
+        {
+            var root = "sqrt(x)".ToEntity();
+            var spelledOut = MathS.Pow("x".ToEntity(), MathS.Pow(2, -1));
+            Assert.True(CostModel.FewestRadicals.Cost(root) >= 1);
+            Assert.True(CostModel.FewestRadicals.Cost(spelledOut) >= 1,
+                "x ^ (2 ^ (-1)) is a square root however the exponent is written");
+        }
+
+        /// <summary>
+        /// And the other half of #950: a model that counts <i>spelling</i> is right to read the
+        /// node as written, so widening the feature models must not have widened these.
+        /// </summary>
+        [Fact]
+        public void ASpellingModelStillCountsHowItIsWritten()
+        {
+            var written = "x / y".ToEntity();
+            var unevaluatedExponent = MathS.Pow("y".ToEntity(), "1".ToEntity() * (-1)) * "x".ToEntity();
+            Assert.True(CostModel.SmallestTree.Cost(written)
+                        < CostModel.SmallestTree.Cost(unevaluatedExponent),
+                "y ^ (1 * (-1)) * x really is a bigger tree, and SmallestTree counts trees");
+        }
+
         /// <summary>They can be listed and named, which is what "as data" buys.</summary>
         [Fact]
         public void TheyCanBeListedAndNamed()