-
Notifications
You must be signed in to change notification settings - Fork 77
Add C to integration result, change integral node to support range instead of iterations, implement u-substitution and quadratic denominator integral, integration of abs(x), sgn(x) and ln(abs(x))^2 #657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request significantly refactors the integral system in AngouriMath by changing from an iteration-based API to a range-based API that distinguishes between indefinite and definite integrals. The PR adds automatic inclusion of the constant of integration (C) for indefinite integrals, implements u-substitution for integration, adds support for integrating rational functions with quadratic denominators, and implements integration of abs(x), sgn(x), and ln(abs(x))^2.
Changes:
- Refactored integral API from
Integral(expr, var, iterations)toIntegral(expr, var)for indefinite integrals andIntegral(expr, var, from, to)for definite integrals - Added automatic constant of integration (C) with unique naming when C is already present in the expression
- Implemented u-substitution algorithm to handle a wide variety of composite function integrals
- Added integration patterns for absolute value, signum function, ln(abs(x)), and rational functions with quadratic denominators
- Updated all output formatters (ToString, LaTeX, SymPy) to handle the new integral representation
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| Sources/Utils/Utils/AntlrPostProcessor.cs | Corrected path to ANTLR files by removing redundant directory level |
| Sources/Tests/UnitTests/PatternsTest/SimplifyTest.cs | Fixed test that incorrectly used Integral instead of Derivative |
| Sources/Tests/UnitTests/Convenience/ToSymPyTEst.cs | Updated SymPy output tests for new integral API |
| Sources/Tests/UnitTests/Convenience/LatexTest.cs | Updated LaTeX tests to use Derivative with negative iterations for integrals |
| Sources/Tests/UnitTests/Convenience/FromStringTest.cs | Updated parsing tests for new 4-argument definite integral syntax |
| Sources/Tests/UnitTests/Common/SubstituteTest.cs | Updated substitution tests for new integral API |
| Sources/Tests/UnitTests/Common/InnerSimplifyTest.cs | Added tests for abs/signum simplifications and updated integration tests to expect C constant |
| Sources/Tests/UnitTests/Common/CircleTest.cs | Updated stringize tests for new integral syntax |
| Sources/Tests/UnitTests/Common/BuiltinFunctionsAppliedTest.cs | Updated curried function tests to expect C constant |
| Sources/Tests/UnitTests/Calculus/IntegrationTest.cs | Extensively expanded with 250+ lines of new integration tests covering u-substitution, quadratic denominators, abs/sgn, and ln(abs(x)) |
| Sources/Tests/UnitTests/Calculus/DerivativeTest.cs | Updated derivative of abs test to expect provided condition |
| Sources/Tests/FSharpWrapperUnitTests/ShortcutsTest.fs | Updated F# wrapper tests to expect C constant |
| Sources/Tests/FSharpWrapperUnitTests/Functions.Order.fs | Updated F# function tests to expect C constant |
| Sources/AngouriMath/Functions/Substitute.cs | Fixed integral substitution to handle range (from, to) properly |
| Sources/AngouriMath/Functions/Simplification/Patterns/Patterns.Common.cs | Moved abs/signum patterns to InnerSimplify and added division simplification patterns with provided conditions |
| Sources/AngouriMath/Functions/Output/ToSympy/ToSympy.Calculus.Classes.cs | Updated SymPy output for range-based integrals |
| Sources/AngouriMath/Functions/Output/ToString/ToString.Calculus.Classes.cs | Updated ToString for range-based integrals |
| Sources/AngouriMath/Functions/Output/Latex/Latex.Calculus.Classes.cs | Updated LaTeX output to render derivatives with negative iterations as integrals and definite integrals with bounds |
| Sources/AngouriMath/Functions/Evaluation/Evaluation.Continuous/Evaluation.Continuous.Trigonometry.Classes.cs | Added arctan evaluation for positive/negative infinity |
| Sources/AngouriMath/Functions/Evaluation/Evaluation.Continuous/Evaluation.Continuous.Calculus.Classes.cs | Refactored integral evaluation to use new range-based API and call Integrate methods |
| Sources/AngouriMath/Functions/Evaluation/Evaluation.Continuous/Evaluation.Continuous.Arithmetics.Classes.cs | Added InnerSimplify patterns for abs(abs), sgn(sgn), abs(sgn), and sgn(abs) |
| Sources/AngouriMath/Functions/Continuous/Solvers/EquationSolver/InvertNode.Classes.cs | Updated equation solving for new integral API |
| Sources/AngouriMath/Functions/Continuous/Limits/Solvers/Limit.Classes.cs | Updated limit computation for range-based integrals |
| Sources/AngouriMath/Functions/Continuous/Integration/Integration.Definition.cs | Refactored to separate indefinite and definite integration, added constant of integration |
| Sources/AngouriMath/Functions/Continuous/Integration/IntegralPatterns.cs | Added patterns for abs, sgn, ln(abs(x)), and quadratic denominators |
| Sources/AngouriMath/Functions/Continuous/Integration/IndefiniteIntegralSolver.cs | Refactored to return nullable results, implemented u-substitution, improved integration by parts |
| Sources/AngouriMath/Functions/Continuous/Differentiation.cs | Added support for negative derivative iterations converting to integrals |
| Sources/AngouriMath/Core/Entity/Discrete/Entity.Discrete.Classes.cs | Fixed comment typo: "state" to "states" |
| Sources/AngouriMath/Core/Entity/Continuous/Entity.Continuous.Calculus.Classes.cs | Changed Integralf from iterations-based to range-based with (from, to) tuple |
| Sources/AngouriMath/Core/Antlr/AngouriMathParser.cs | Updated parser to accept 4-argument integral for definite integrals |
| Sources/AngouriMath/Core/Antlr/AngouriMath.g | Updated grammar to accept 4-argument integral and updated documentation comment |
| Sources/AngouriMath/Convenience/MathS.cs | Updated Integral factory methods for new API and added Boolean.True/False constants |
| Sources/AngouriMath/Convenience/AngouriMathExtensions.cs | Added definite integral extension method and fixed spelling errors |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| using GenericTensor.Core; | ||
| using System.Linq.Expressions; |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary imports added. The using statements for GenericTensor.Core and System.Linq.Expressions are added but don't appear to be used anywhere in the file based on the diff. These should be removed to keep the code clean.
| using GenericTensor.Core; | |
| using System.Linq.Expressions; |
| // Details: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md. | ||
| // Website: https://am.angouri.org. | ||
| // | ||
| using HonkSharp.Fluency; |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing using directive. The code uses HonkSharp.Fluency for the .Pipe() extension method but this dependency should be verified to exist in the project. If this is a new dependency, it should be documented or the code should be refactored to not require it.
| /// </summary> | ||
| /// <param name="str"> | ||
| /// The expression to be parsed and integrated over <paramref name="x"/> | ||
| /// The expresion to be parsed and integrated |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling error: "expresion" should be "expression".
| namespace AngouriMath.Functions.Algebra | ||
| { | ||
| internal static class IndefiniteIntegralSolver | ||
| { |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition integrateByParts is passed through to recursive calls in SolveBySplittingSum but is not documented. This parameter prevents infinite recursion during integration by parts, but the purpose and behavior should be documented to help maintainers understand the control flow.
| { | |
| { | |
| /// <summary> | |
| /// Attempts to integrate an expression that is a sum by splitting it into additive terms, | |
| /// integrating each term independently, and then summing the resulting antiderivatives. | |
| /// </summary> | |
| /// <param name="expr">The expression to integrate.</param> | |
| /// <param name="x">The variable of integration.</param> | |
| /// <param name="integrateByParts"> | |
| /// Controls whether integration by parts is allowed for this call chain. The flag is | |
| /// passed through to recursive calls so that, once integration by parts has been applied | |
| /// in a particular branch of the algorithm, it can be disabled for sub-expressions to | |
| /// avoid uncontrolled or infinite recursion on similar patterns. | |
| /// </param> | |
| /// <returns> | |
| /// The antiderivative of <paramref name="expr"/> with respect to <paramref name="x"/> if | |
| /// all terms of the sum can be integrated by the underlying solver; otherwise, <c>null</c>. | |
| /// </returns> |
| /// May return an unresolved <see cref="Integralf"/> node. | ||
| /// </summary> | ||
| /// <param name="str"> | ||
| /// The expresion to be parsed and integrated |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling error: "expresion" should be "expression".
| // Website: https://am.angouri.org. | ||
| // | ||
| using HonkSharp.Fluency; | ||
| using static AngouriMath.Entity; |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing using directive. The code uses using static AngouriMath.Entity; but then still refers to Entity.Variable, Entity.Mulf, etc. This import appears incomplete or unnecessary - either fully utilize the static import or remove it for consistency.
| using static AngouriMath.Entity; |
| public static class AntlrPostProcessorReplacePublicWithInternal | ||
| { | ||
| public const string ANTLR_PATH = "../AngouriMath/AngouriMath/Core/Antlr/"; | ||
| public const string ANTLR_PATH = "../AngouriMath/Core/Antlr/"; |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The path has been changed from ../AngouriMath/AngouriMath/Core/Antlr/ to ../AngouriMath/Core/Antlr/. This appears to be a correction of a redundant directory level. Ensure that this path change is correct and that the build/test infrastructure can find the files at this new location.
| var expr = x; | ||
|
|
||
| Assert.True((MathS.Compute.DefiniteIntegral(expr, x, 0, 1).RealPart - 1.0/2).Abs() < 0.1); | ||
| Assert.True((MathS.Compute.DefiniteIntegral(expr, x, 0, 1).RealPart - 1.0 / 2).Abs() < 0.1); |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call to obsolete method DefiniteIntegral.
| return changed; | ||
| } | ||
|
|
||
| private static Entity? ConditionallySimplified(Entity e, bool isExact) => e is Integralf i ? null : e.InnerSimplified(isExact); |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assignment to i is useless, since its value is never read.
| private static Entity? ConditionallySimplified(Entity e, bool isExact) => e is Integralf i ? null : e.InnerSimplified(isExact); | |
| private static Entity? ConditionallySimplified(Entity e, bool isExact) => e is Integralf ? null : e.InnerSimplified(isExact); |
| if (_localctx.args.list.Count == 4) | ||
| _localctx.value = MathS.Integral(_localctx.args.list[0], _localctx.args.list[1], _localctx.args.list[2], _localctx.args.list[3]); | ||
| else | ||
| throw new InvalidArgumentParseException("Expected number for the third argument of integral"); | ||
| _localctx.value = MathS.Integral(_localctx.args.list[0], _localctx.args.list[1]); |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
No description provided.