diff --git a/BREAKING-CHANGES.md b/BREAKING-CHANGES.md index 3085a0914..8df4bc356 100644 --- a/BREAKING-CHANGES.md +++ b/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 diff --git a/Sources/AngouriMath/Core/CostModel.cs b/Sources/AngouriMath/Core/CostModel.cs index 42c24de8a..21e08dc83 100644 --- a/Sources/AngouriMath/Core/CostModel.cs +++ b/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 now holds. /// /// + /// A model counts one of two things, and which one decides what it may read. + /// and count how the expression is + /// written: y ^ (1 * (-1)) * x really is a bigger tree than x / y, so + /// reading the node as written is not an approximation, it is the question. The others count a + /// mathematical feature — 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 + /// to rather than what it was typed as. + /// + /// + /// The distinction is worth stating because getting it wrong is silent and backwards: + /// used to test the exponent's node, so + /// y ^ (1 * (-1)) * x — a division written where the test could not see it — scored + /// cheaper than x / y, under the one criterion whose whole job is to remove + /// divisions. That is + /// #950. A caller + /// writing their own model wants to decide which of the two kinds it is, 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. + /// + /// /// Every model here counts nodes a little, even the ones that are about something else. /// 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, FuncPrefers the expression with fewest divisions, then fewest nodes. /// /// 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: y ^ (1 * (-1)) is a division however it is spelled, + /// and a test that missed it would rate the unevaluated writing cheapest. /// 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 })); /// Prefers the expression with fewest radicals, then fewest nodes. /// /// A radical is a power by a non-integer rational — sqrt(x) is x ^ (1/2) - /// 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 + /// x ^ (2 ^ (-1)) counts as the radical it is. /// 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)); /// /// Every model here, so a caller can offer the choice rather than hard-code one. diff --git a/Sources/Tests/UnitTests/Core/CostModelTest.cs b/Sources/Tests/UnitTests/Core/CostModelTest.cs index 9ebacbfcd..94e962742 100644 --- a/Sources/Tests/UnitTests/Core/CostModelTest.cs +++ b/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)); } + /// + /// 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. + /// + [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( + Assert.IsType(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"); + } + + /// The same blind spot, in the other feature model. + [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"); + } + + /// + /// And the other half of #950: a model that counts spelling is right to read the + /// node as written, so widening the feature models must not have widened these. + /// + [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"); + } + /// They can be listed and named, which is what "as data" buys. [Fact] public void TheyCanBeListedAndNamed()