Skip to content

Conversation

@khanhkhanhlele
Copy link
Contributor

@khanhkhanhlele khanhkhanhlele commented Nov 11, 2025

This PR adds method SinFunction.
Contribution by Gittensor, learn more at https://gittensor.io/

Copilot AI review requested due to automatic review settings November 11, 2025 03:07
Copilot finished reviewing on behalf of khanhkhanhlele November 11, 2025 03:09
Copy link

Copilot AI left a 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 PR adds a new Sin class that calculates the sine function using Maclaurin series expansion. The implementation includes angle reduction for better convergence and uses an iterative approach to compute the series terms efficiently.

  • Introduces Sin.Calculate() method with configurable term count (default 20)
  • Adds comprehensive test suite covering common angles, edge cases, and error handling
  • Implements angle reduction (modulo 2π) for improved series convergence

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
Algorithms/Numeric/Sin.cs New standalone Sin class with Maclaurin series implementation using iterative term calculation
Algorithms.Tests/Numeric/SinTests.cs Comprehensive test suite with common angles, negative values, large angle reduction, minimal terms, and exception handling tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +2 to +3
using System.Numerics;

Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The System.Numerics namespace is imported but never used in this file. Remove this unused import to keep the code clean.

Suggested change
using System.Numerics;

Copilot uses AI. Check for mistakes.
Comment on lines +4 to +66
namespace Algorithms.Numeric;

/// <summary>
/// Calculates the sine function (sin(x)) using the Maclaurin series expansion (Taylor series centered at 0).
/// sin(x) = x - x^3/3! + x^5/5! - x^7/7! + ...
/// </summary>
public static class Sin
{
/// <summary>
/// Calculates the sine of an angle x using a finite number of terms from the Maclaurin series.
/// Note: For high precision, use Math.Sin(). This method is for demonstrating the series expansion.
/// </summary>
/// <param name="x">The angle in radians.</param>
/// <param name="terms">The number of terms (odd powers) to include in the series. Default is 20 for required precision.</param>
/// <returns>The approximate value of sin(x).</returns>
public static double Calculate(double x, int terms = 20) // Increased default from 15 to 20 for higher precision
{
if (terms <= 0)
{
throw new ArgumentException("The number of terms must be a positive integer.");
}

// Best practice for series convergence: reduce the angle to the range [-2*PI, 2*PI].
// The series converges for all x, but convergence is faster near 0.
x %= 2 * Math.PI;

double result = 0.0;
double xSquared = x * x;

// currentPower: Stores x^k
double currentPower = x; // Starts at x^1

// currentFactorial: Stores k!
double currentFactorial = 1.0; // Starts at 1!

int sign = 1; // Starts positive

// k is the exponent/factorial index (1, 3, 5, 7, ...)
for (int i = 0, k = 1; i < terms; i++, k += 2)
{
if (k > 1)
{
// Iteratively update the components for the next term:
// Current exponent k is (k-2) + 2.
// x^k = x^(k-2) * x^2
currentPower *= xSquared;

// k! = (k-2)! * (k-1) * k
// Example: 5! = 3! * 4 * 5
currentFactorial *= (k - 1) * k;
}

// Calculate the term: (sign) * x^k / k!
double term = currentPower / currentFactorial;

result += sign * term;

sign *= -1; // Alternate the sign for the next term
}

return result;
}
}
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This codebase already has a Maclaurin.Sin() method in Algorithms/Numeric/Series/Maclaurin.cs that implements the same mathematical operation. Consider whether this new implementation provides distinct value or if it creates unnecessary duplication. If both are needed, consider documenting why two implementations exist and when to use each.

Suggested change
namespace Algorithms.Numeric;
/// <summary>
/// Calculates the sine function (sin(x)) using the Maclaurin series expansion (Taylor series centered at 0).
/// sin(x) = x - x^3/3! + x^5/5! - x^7/7! + ...
/// </summary>
public static class Sin
{
/// <summary>
/// Calculates the sine of an angle x using a finite number of terms from the Maclaurin series.
/// Note: For high precision, use Math.Sin(). This method is for demonstrating the series expansion.
/// </summary>
/// <param name="x">The angle in radians.</param>
/// <param name="terms">The number of terms (odd powers) to include in the series. Default is 20 for required precision.</param>
/// <returns>The approximate value of sin(x).</returns>
public static double Calculate(double x, int terms = 20) // Increased default from 15 to 20 for higher precision
{
if (terms <= 0)
{
throw new ArgumentException("The number of terms must be a positive integer.");
}
// Best practice for series convergence: reduce the angle to the range [-2*PI, 2*PI].
// The series converges for all x, but convergence is faster near 0.
x %= 2 * Math.PI;
double result = 0.0;
double xSquared = x * x;
// currentPower: Stores x^k
double currentPower = x; // Starts at x^1
// currentFactorial: Stores k!
double currentFactorial = 1.0; // Starts at 1!
int sign = 1; // Starts positive
// k is the exponent/factorial index (1, 3, 5, 7, ...)
for (int i = 0, k = 1; i < terms; i++, k += 2)
{
if (k > 1)
{
// Iteratively update the components for the next term:
// Current exponent k is (k-2) + 2.
// x^k = x^(k-2) * x^2
currentPower *= xSquared;
// k! = (k-2)! * (k-1) * k
// Example: 5! = 3! * 4 * 5
currentFactorial *= (k - 1) * k;
}
// Calculate the term: (sign) * x^k / k!
double term = currentPower / currentFactorial;
result += sign * term;
sign *= -1; // Alternate the sign for the next term
}
return result;
}
}
// Removed duplicate Sin class. Use Maclaurin.Sin() in Algorithms/Numeric/Series/Maclaurin.cs for sine calculation via Maclaurin series.

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Nov 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.89%. Comparing base (5bcbece) to head (6164b17).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #568   +/-   ##
=======================================
  Coverage   96.89%   96.89%           
=======================================
  Files         291      292    +1     
  Lines       12035    12058   +23     
  Branches     1740     1743    +3     
=======================================
+ Hits        11661    11684   +23     
  Misses        237      237           
  Partials      137      137           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI review requested due to automatic review settings November 11, 2025 03:12
Copilot finished reviewing on behalf of khanhkhanhlele November 11, 2025 03:12
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@siriak
Copy link
Member

siriak commented Nov 11, 2025

I think it's already implemented here

public static class Maclaurin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants