The integrator stops answering the same question over and over (#1156)

Sources/.editorconfig

@@ -192,3 +192,6 @@ file_header_template=\nCopyright (c) 2019-2026 Angouri.\nAngouriMath is licensed
 
 [Tests/UnitTests/Calculus/RepeatedQuadraticIntegralTest.cs]
 file_header_template=\nCopyright (c) 2019-2026 Angouri.\nAngouriMath is licensed under MIT.\nDetails: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md.\nWebsite: https://am.angouri.org.\n
+
+[Tests/UnitTests/Calculus/IntegralAnswerCacheTest.cs]
+file_header_template=\nCopyright (c) 2019-2026 Angouri.\nAngouriMath is licensed under MIT.\nDetails: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md.\nWebsite: https://am.angouri.org.\n

Sources/AngouriMath/Convenience/SettingClass.cs

@@ -10,6 +10,88 @@
 
 namespace AngouriMath.Convenience
 {
+    /// <summary>A setting, without its value's type — enough to ask what it currently reads as.</summary>
+    internal interface ISettingState
+    {
+        /// <summary>
+        /// The object identifying this setting's value in the calling flow, or
+        /// <see langword="null"/> where nothing is set and the default applies.
+        /// </summary>
+        object? CurrentState { get; }
+    }
+
+    /// <summary>
+    /// What every setting reads as, taken together, so that something holding a result computed
+    /// under the settings of a moment can tell whether those settings still hold.
+    /// </summary>
+    /// <remarks>
+    /// <para>
+    /// <b>A state and not a change count.</b> Counting changes looks equivalent and is not: a
+    /// scope opened and closed leaves every setting exactly as it found it while moving a
+    /// counter twice, and the library opens such scopes constantly — a single
+    /// <c>Simplify</c> of one unevaluated integral opened <b>3520</b> of them, all four from
+    /// numeric downcasting in <c>Number/Operators.cs</c>. A counter therefore reports
+    /// "everything has changed" continuously and is useless to a cache. Measured, not assumed:
+    /// the counter was built first and threw away the whole benefit.
+    /// </para>
+    /// <para>
+    /// The state of one setting is the <i>reference</i> to the frame on top of its stack, which
+    /// is exact rather than a hash — releasing a scope restores the very frame object that was
+    /// there before it, since the chain below a pushed frame is never rebuilt. So comparing
+    /// snapshots is a handful of reference comparisons with no chance of collision, and a
+    /// balanced open-and-close compares equal, which is the whole point.
+    /// </para>
+    /// </remarks>
+    internal static class SettingsState
+    {
+        /// <summary>
+        /// Every setting there is. Replaced rather than appended to, so that a reader can walk it
+        /// without a lock: registering happens as the settings are constructed and a reader is on
+        /// the integrator's hot path, where taking a lock per call would be contention bought for
+        /// a list that stops changing before any real work starts.
+        /// </summary>
+        [ConcurrentField] private static ISettingState[] registered = System.Array.Empty<ISettingState>();
+
+        [ConcurrentField] private static readonly object registering = new();
+
+        internal static void Register(ISettingState setting)
+        {
+            lock (registering)
+            {
+                var grown = new ISettingState[registered.Length + 1];
+                System.Array.Copy(registered, grown, registered.Length);
+                grown[^1] = setting;
+                System.Threading.Volatile.Write(ref registered, grown);
+            }
+        }
+
+        /// <summary>What every setting reads as right now, in this flow.</summary>
+        internal static object?[] Snapshot()
+        {
+            var settings = System.Threading.Volatile.Read(ref registered);
+            var snapshot = new object?[settings.Length];
+            for (var i = 0; i < snapshot.Length; i++)
+                snapshot[i] = settings[i].CurrentState;
+            return snapshot;
+        }
+
+        /// <summary>
+        /// Whether <paramref name="snapshot"/> is still what every setting reads as. One taken
+        /// before a setting existed has a different length and is reported stale, which is what
+        /// should happen — that setting's value was not part of it.
+        /// </summary>
+        internal static bool StillHolds(object?[] snapshot)
+        {
+            var settings = System.Threading.Volatile.Read(ref registered);
+            if (snapshot.Length != settings.Length)
+                return false;
+            for (var i = 0; i < snapshot.Length; i++)
+                if (!ReferenceEquals(snapshot[i], settings[i].CurrentState))
+                    return false;
+            return true;
+        }
+    }
+
     /// <summary>
     /// This class for configuring some internal mechanisms from outside
     /// </summary>
@@ -34,7 +116,7 @@ namespace AngouriMath.Convenience
     /// chain and assign it, rather than editing one.
     /// </para>
     /// </remarks>
-    public sealed class Setting<T> where T : notnull
+    public sealed class Setting<T> : ISettingState where T : notnull
     {
         /// <summary>
         /// One pushed value, and the chain below it. Never mutated once built.
@@ -58,7 +140,20 @@ private sealed class Frame
         private readonly AsyncLocal<Frame?> frames = new();
         private long lastId;
 
-        internal Setting(T defaultValue) => Default = defaultValue;
+        internal Setting(T defaultValue)
+        {
+            Default = defaultValue;
+            SettingsState.Register(this);
+        }
+
+        /// <summary>
+        /// The frame this setting currently reads from, as the identity of its state. Released
+        /// in order — which <c>using</c> guarantees — a scope restores the very frame that was
+        /// on top before it, so this compares equal across a balanced open and close. Out of
+        /// order the chain above is rebuilt and this reads as a change, which is conservative
+        /// and not wrong.
+        /// </summary>
+        object? ISettingState.CurrentState => frames.Value;
 
         /// <summary>
         /// Sets the new value for the setting
@@ -98,7 +193,7 @@ private void Remove(long id)
             if (top.Id == id)
             {
                 frames.Value = top.Next;
-                return;
+                    return;
             }
             var above = new System.Collections.Generic.List<Frame>();
             var current = top;

Sources/AngouriMath/Functions/Continuous/Integration/Integration.Definition.cs

@@ -5,6 +5,7 @@
 // Website: https://am.angouri.org.
 //
 
+using AngouriMath.Convenience;
 using AngouriMath.Core.Transformations;
 using AngouriMath.Extensions;
 using AngouriMath.Functions.Algebra;
@@ -109,10 +110,98 @@ internal static partial class Integration
         private static Entity Normalized(Entity expr) =>
             expr.Replace(Patterns.GatherPowersOfOneBase);
 
+        /// <summary>
+        /// The integrals already answered under the settings in force, so that the same question
+        /// is not worked out again.
+        /// </summary>
+        /// <remarks>
+        /// <para>
+        /// <b>Why an integral is asked for twice at all.</b> Two layers of it. Within one call the
+        /// solvers overlap — substitution, partial fractions, splitting a sum and integration by
+        /// parts each decompose the integrand differently and produce pieces that coincide — and
+        /// across calls <c>Simplify</c> asks about one integral through every
+        /// rewritten candidate it generates. Traced on <c>sin(x)/(x^2 + 1)^2</c>, which has no
+        /// elementary antiderivative and so runs the search to exhaustion, that is <b>562
+        /// top-level calls for 3 distinct integrands</b>, one of them asked 500 times; on
+        /// <c>e^x/(x^2 + 1)^2</c> a single call enters the integrator <b>5330 times for 23</b>.
+        /// </para>
+        /// <para>
+        /// <b>The answers are held across calls and not only within one</b>, which is where the
+        /// repetition is: discarding them at the end of each top-level call leaves the 562 to be
+        /// paid in full. Measured on those three integrands, keeping them takes a
+        /// <c>Simplify</c> from 115, 86 and 51 seconds to about 1.2, 1.5 and 0.2, and a single
+        /// cold <c>Integrate</c> of <c>sqrt(tan(x))</c> from 155 ms to 79.
+        /// https://github.com/asc-community/AngouriMath/issues/1156
+        /// </para>
+        /// <para>
+        /// <b>Why it is sound to hold them.</b> An answer depends on the ambient settings —
+        /// <see cref="MathS.Settings.Codomain"/> decides whether the logarithms carry an
+        /// <c>abs</c>, <c>MaxExpansionTermCount</c> bounds the by-parts recursion — and a setting
+        /// is scoped to a flow rather than to a thread, so "the settings have not changed" is not
+        /// something a per-thread cache can assume. <see cref="SettingsState"/> answers it
+        /// exactly, by comparing what every setting reads as rather than by counting changes —
+        /// which matters, because the library opens and closes thousands of balanced scopes while
+        /// simplifying and a change count is therefore never still.
+        /// </para>
+        /// </remarks>
+        [System.ThreadStatic] private static Dictionary<(Entity, Entity.Variable, bool), Entity?>? answered;
+
+        /// <summary>The settings <see cref="answered"/> was filled under.</summary>
+        [System.ThreadStatic] private static object?[]? answeredUnder;
+
+        /// <summary>
+        /// How many answers to hold before starting over. Comfortably above what one call needs
+        /// — the worst measured is 23 distinct integrands within a call and 3 across the
+        /// candidates of one <c>Simplify</c> — and low enough that a process integrating
+        /// different things all day does not accumulate them.
+        /// </summary>
+        private const int MostAnswersKept = 4096;
+
         /// <summary>Does not add the constant of integration because this is called recursively.</summary>
         internal static Entity? ComputeIndefiniteIntegral(Entity expr, Entity.Variable x, bool integrateByParts = true)
         {
             expr = Normalized(expr);
+
+            if (answered is null || answeredUnder is null || !SettingsState.StillHolds(answeredUnder))
+            {
+                // A fresh dictionary rather than Clear(), because this flow may have inherited
+                // the reference from the one that started it, and emptying it in place would
+                // empty that one's too.
+                answered = new();
+                answeredUnder = SettingsState.Snapshot();
+            }
+
+            // Held as locals over the recursive call, which reaches this method again and may
+            // replace both fields on the way. Writing the answer into whatever the fields hold
+            // afterwards would put it in a dictionary stamped with different settings.
+            var into = answered;
+            var stamp = answeredUnder;
+
+            var key = (expr, x, integrateByParts);
+            if (into.TryGetValue(key, out var already))
+                return already;
+
+            var computed = ComputeIndefiniteIntegralUncached(expr, x, integrateByParts);
+
+            // Only kept if those settings still hold. Working the answer out runs simplification,
+            // which opens scopes of its own; one still open means this answer was computed under
+            // settings the stamp does not describe.
+            if (SettingsState.StillHolds(stamp))
+            {
+                // Emptied rather than grown without end. Nothing here expires on its own — the
+                // settings holding still is the whole condition for keeping an answer — so a
+                // long-lived process integrating a stream of different expressions would hold
+                // every one of them for ever. What this is for is being asked the same question
+                // again, which needs the recent answers and not all of them.
+                if (into.Count >= MostAnswersKept)
+                    into.Clear();
+                into[key] = computed;
+            }
+            return computed;
+        }
+
+        private static Entity? ComputeIndefiniteIntegralUncached(Entity expr, Entity.Variable x, bool integrateByParts)
+        {
             if (!expr.ContainsNode(x)) return expr * x; // base case, handle here
             if ((IntegralPatterns.TryStandardIntegrals(expr, x)) is { } answer) return answer;
             // The flag has to be handed on. Every one of these recurses, and a solver that

Sources/Tests/UnitTests/Calculus/IntegralAnswerCacheTest.cs

@@ -0,0 +1,119 @@
+//
+// Copyright (c) 2019-2026 Angouri.
+// AngouriMath is licensed under MIT.
+// Details: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md.
+// Website: https://am.angouri.org.
+//
+
+using System.Diagnostics;
+using AngouriMath;
+using AngouriMath.Core;
+using AngouriMath.Extensions;
+using Xunit;
+
+namespace AngouriMath.Tests.Calculus
+{
+    /// <summary>
+    /// The integrator keeps the answers it has already worked out, because it is asked the same
+    /// question over and over — <c>Simplify</c> puts one integral through every rewritten
+    /// candidate it generates, which measured at 562 calls for 3 distinct integrands.
+    /// https://github.com/asc-community/AngouriMath/issues/1156
+    /// </summary>
+    /// <remarks>
+    /// The tests that matter here are the ones below on the codomain. A cache like this does not
+    /// fail by returning nonsense; it fails by returning a <i>correct</i> answer that was worked
+    /// out under settings the caller is no longer standing in.
+    /// </remarks>
+    [Trait("Area", "Calculus")]
+    public sealed class IntegralAnswerCacheTest
+    {
+        /// <summary>
+        /// <c>1/x</c> integrates to <c>ln(abs(x))</c> on the reals and <c>ln(x)</c> on the complex
+        /// plane, so it separates the two codomains in one expression. Asked under each in turn,
+        /// in one flow, which is where a cache that does not notice the setting would answer the
+        /// second question with the first one's answer.
+        /// </summary>
+        [Fact]
+        public void ANarrowedCodomainIsNotAnsweredFromTheWiderOne()
+        {
+            var complex = "1 / x".ToEntity().Integrate("x").Stringize();
+
+            string real;
+            using (MathS.Settings.Codomain.Set(Domain.Real))
+                real = "1 / x".ToEntity().Integrate("x").Stringize();
+
+            Assert.Contains("abs", real);
+            Assert.DoesNotContain("abs", complex);
+        }
+
+        /// <summary>The same, asked in the other order — a cache is only sound in both.</summary>
+        [Fact]
+        public void AWidenedCodomainIsNotAnsweredFromTheNarrowerOne()
+        {
+            string real;
+            using (MathS.Settings.Codomain.Set(Domain.Real))
+                real = "1 / x".ToEntity().Integrate("x").Stringize();
+
+            var complex = "1 / x".ToEntity().Integrate("x").Stringize();
+
+            Assert.Contains("abs", real);
+            Assert.DoesNotContain("abs", complex);
+        }
+
+        /// <summary>
+        /// And with the scope entered while an answer for the same integrand is already held,
+        /// which is the ordering that a cache keyed on anything coarser than the settings
+        /// themselves gets wrong.
+        /// </summary>
+        [Fact]
+        public void AnAnswerHeldFromBeforeAScopeIsNotUsedInsideIt()
+        {
+            var before = "1 / x".ToEntity().Integrate("x").Stringize();
+            Assert.DoesNotContain("abs", before);
+
+            using (MathS.Settings.Codomain.Set(Domain.Real))
+                Assert.Contains("abs", "1 / x".ToEntity().Integrate("x").Stringize());
+
+            Assert.DoesNotContain("abs", "1 / x".ToEntity().Integrate("x").Stringize());
+        }
+
+        /// <summary>
+        /// Repeating a question gives the same answer, which is the property a cache is allowed
+        /// to have and the one that would break first if a stale entry were served.
+        /// </summary>
+        [Theory]
+        [InlineData("1 / (x ^ 2 + 1)")]
+        [InlineData("1 / (x ^ 2 + 1) ^ 2")]
+        [InlineData("x * ln(x)")]
+        [InlineData("sqrt(tan(x))")]
+        [InlineData("sin(x) / (x ^ 2 + 1) ^ 2")]
+        public void AskingTwiceAnswersTheSame(string integrand)
+        {
+            var first = integrand.ToEntity().Integrate("x");
+            var second = integrand.ToEntity().Integrate("x");
+            Assert.Equal(first, second);
+        }
+
+        /// <summary>
+        /// An integrand with no elementary antiderivative, which runs every solver to exhaustion
+        /// and so pays the full cost of the search. It took <b>115 seconds</b> before the answers
+        /// were kept, and about one after.
+        /// </summary>
+        /// <remarks>
+        /// The bound is two orders of magnitude above what it now takes and a quarter of what it
+        /// used to, so it is a guard against the regression rather than a measurement of the
+        /// machine. A timing assertion tight enough to be a benchmark would only flake.
+        /// </remarks>
+        [Fact]
+        public void AnIntegralWithNoAnswerStillFinishesQuickly()
+        {
+            var watch = Stopwatch.StartNew();
+            var answer = "sin(x) / (x ^ 2 + 1) ^ 2".ToEntity().Integrate("x").Simplify();
+            watch.Stop();
+
+            Assert.Contains("integral(", answer.Stringize());
+            Assert.True(watch.Elapsed.TotalSeconds < 30,
+                $"took {watch.Elapsed.TotalSeconds:F1}s; it was 115s before the answers were kept");
+        }
+    }
+}