Skip to content
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

Add Fisher Transform indicator #49

Merged
merged 2 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions Tests/test_updates_oscillators.cs
Original file line number Diff line number Diff line change
Expand Up @@ -321,4 +321,19 @@ public void Dosc_Update()

Assert.Equal(initialValue, finalValue, precision);
}

[Fact]
public void Fisher_Update()
{
var indicator = new Fisher(period: 10);
double initialValue = indicator.Calc(new TValue(DateTime.Now, ReferenceValue, IsNew: true));

for (int i = 0; i < RandomUpdates; i++)
{
indicator.Calc(new TValue(DateTime.Now, GetRandomDouble(), IsNew: false));
}
Comment on lines +167 to +170
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Consider adding edge case tests.

While the random update testing is good, consider adding specific test cases for:

  • Extreme values (very high/low)
  • Rapid value changes
  • Constant values

Example addition:

// Test extreme values
indicator.Calc(new TValue(DateTime.Now, double.MaxValue * 0.1, IsNew: false));
indicator.Calc(new TValue(DateTime.Now, -double.MaxValue * 0.1, IsNew: false));

// Test constant values
for (int i = 0; i < 10; i++)
{
    indicator.Calc(new TValue(DateTime.Now, 100.0, IsNew: false));
}

double finalValue = indicator.Calc(new TValue(DateTime.Now, ReferenceValue, IsNew: false));

Assert.Equal(initialValue, finalValue, precision);
}
Comment on lines +162 to +174
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Add documentation for the Fisher Transform test.

Consider adding a brief XML documentation comment explaining:

  • The purpose of the Fisher Transform indicator
  • Why period=10 was chosen as the test value
  • Expected behavior with the reference value

Example:

/// <summary>
/// Verifies that the Fisher Transform indicator maintains consistency
/// when receiving repeated reference values after random updates.
/// Period of 10 is used as it's the commonly recommended value for this indicator.
/// </summary>
[Fact]
public void Fisher_Update()

}
95 changes: 95 additions & 0 deletions lib/oscillators/Fisher.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
using System.Runtime.CompilerServices;
namespace QuanTAlib;

/// <summary>
/// FISHER: Fisher Transform
/// A technical indicator that converts prices into a Gaussian normal distribution.
/// </summary>
/// <remarks>
/// The Fisher Transform calculation process:
/// 1. Calculate the value of the price relative to its high-low range.
/// 2. Apply the Fisher Transform formula to the normalized price.
/// 3. Smooth the result using an exponential moving average.
///
/// Key characteristics:
/// - Oscillates between -1 and 1
/// - Emphasizes price reversals
/// - Can be used to identify overbought and oversold conditions
///
/// Formula:
/// Fisher Transform = 0.5 * log((1 + x) / (1 - x))
/// where:
/// x = 2 * ((price - min) / (max - min) - 0.5)
///
/// Sources:
/// John F. Ehlers - "Rocket Science for Traders" (2001)
/// https://www.investopedia.com/terms/f/fisher-transform.asp
/// </remarks>
[SkipLocalsInit]
public sealed class Fisher : AbstractBase
{
private readonly int _period;
private readonly double[] _prices;
private double _prevFisher;
private double _prevValue;

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / CodeQL

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

Remove the unused private field '_prevValue'. (https://rules.sonarsource.com/csharp/RSPEC-1144)

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

Remove the unused private field '_prevValue'. (https://rules.sonarsource.com/csharp/RSPEC-1144)

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

Remove the unused private field '_prevValue'. (https://rules.sonarsource.com/csharp/RSPEC-1144)

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

Remove the unused private field '_prevValue'. (https://rules.sonarsource.com/csharp/RSPEC-1144)

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

Remove the unused private field '_prevValue'. (https://rules.sonarsource.com/csharp/RSPEC-1144)

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

Remove the unused private field '_prevValue'. (https://rules.sonarsource.com/csharp/RSPEC-1144)

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

The field 'Fisher._prevValue' is never used

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

Remove the unused private field '_prevValue'. (https://rules.sonarsource.com/csharp/RSPEC-1144)

Check warning on line 34 in lib/oscillators/Fisher.cs

View workflow job for this annotation

GitHub Actions / Code_Coverage

Remove the unused private field '_prevValue'. (https://rules.sonarsource.com/csharp/RSPEC-1144)
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Remove unused field '_prevValue'

The field _prevValue is declared but never used in the class. Removing it will clean up the code and eliminate unnecessary clutter.

Apply this diff to remove the unused field:

 private readonly int _period;
 private readonly double[] _prices;
-private double _prevValue;
 private double _prevFisher;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private double _prevValue;
private readonly int _period;
private readonly double[] _prices;
private double _prevFisher;
🧰 Tools
🪛 GitHub Check: CodeQL

[warning] 34-34:
The field 'Fisher._prevValue' is never used


[warning] 34-34:
The field 'Fisher._prevValue' is never used


[warning] 34-34:
The field 'Fisher._prevValue' is never used


[warning] 34-34:
The field 'Fisher._prevValue' is never used


[warning] 34-34:
The field 'Fisher._prevValue' is never used


[warning] 34-34:
The field 'Fisher._prevValue' is never used


[warning] 34-34:
The field 'Fisher._prevValue' is never used


[warning] 34-34:
The field 'Fisher._prevValue' is never used


/// <param name="source">The data source object that publishes updates.</param>
/// <param name="period">The calculation period (default: 10)</param>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public Fisher(object source, int period = 10) : this(period)
{
var pubEvent = source.GetType().GetEvent("Pub");
pubEvent?.AddEventHandler(source, new ValueSignal(Sub));
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public Fisher(int period = 10)
{
_period = period;
_prices = new double[period];
WarmupPeriod = period;
Name = "FISHER";
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
protected override void ManageState(bool isNew)
{
if (isNew)
{
_index++;
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private double NormalizePrice(double price, double min, double max)
{
return 2 * ((price - min) / (max - min) - 0.5);

Check notice on line 66 in lib/oscillators/Fisher.cs

View check run for this annotation

codefactor.io / CodeFactor

lib/oscillators/Fisher.cs#L66

Arithmetic expressions should declare precedence. (SA1407)
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Clarify arithmetic precedence with additional parentheses

Adding parentheses can make the arithmetic operations clearer and enhance code readability, even if the current expression is technically correct.

Apply this diff to add parentheses for clarity:

 return 2 * ((price - min) / (max - min) - 0.5);
+return 2 * (((price - min) / (max - min)) - 0.5);

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 GitHub Check: CodeFactor

[notice] 66-66: lib/oscillators/Fisher.cs#L66
Arithmetic expressions should declare precedence. (SA1407)


⚠️ Potential issue

Handle potential division by zero when 'max' equals 'min' in 'NormalizePrice'

In the NormalizePrice method, if max equals min, the denominator (max - min) becomes zero, leading to a division by zero error. This situation can occur when all prices in the period are the same.

Apply this diff to handle the scenario where max equals min:

 private double NormalizePrice(double price, double min, double max)
 {
+    var range = max - min;
+    if (range == 0)
+        return 0;
+    else
+        return 2 * ((price - min) / range - 0.5);
-    return 2 * ((price - min) / (max - min) - 0.5);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return 2 * ((price - min) / (max - min) - 0.5);
private double NormalizePrice(double price, double min, double max)
{
var range = max - min;
if (range == 0)
return 0;
else
return 2 * ((price - min) / range - 0.5);
}
🧰 Tools
🪛 GitHub Check: CodeFactor

[notice] 66-66: lib/oscillators/Fisher.cs#L66
Arithmetic expressions should declare precedence. (SA1407)

}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private double FisherTransform(double value)
{
return 0.5 * System.Math.Log((1 + value) / (1 - value));
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Prevent division by zero or invalid logarithm in 'FisherTransform'

In the FisherTransform method, if value equals 1 or -1, the denominator (1 - value) becomes zero, and the logarithm function receives an undefined or infinite input. To prevent exceptions, clamp value to be within the open interval (-0.999, 0.999).

Apply this diff to clamp value within a safe range:

 private double FisherTransform(double value)
 {
+    const double limit = 0.999;
+    if (value >= limit) value = limit;
+    if (value <= -limit) value = -limit;
     return 0.5 * System.Math.Log((1 + value) / (1 - value));
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return 0.5 * System.Math.Log((1 + value) / (1 - value));
const double limit = 0.999;
if (value >= limit) value = limit;
if (value <= -limit) value = -limit;
return 0.5 * System.Math.Log((1 + value) / (1 - value));

}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
protected override double Calculation()
{
ManageState(Input.IsNew);

var idx = _index % _period;
_prices[idx] = Input.Value;

if (_index < _period - 1) return double.NaN;

var min = _prices.Min();
var max = _prices.Max();
var normalizedPrice = NormalizePrice(Input.Value, min, max);
var fisherValue = FisherTransform(normalizedPrice);

var smoothedFisher = 0.5 * (fisherValue + _prevFisher);
_prevFisher = smoothedFisher;

return smoothedFisher;
}
}
4 changes: 2 additions & 2 deletions lib/oscillators/_list.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Oscillators indicators
Done: 21, Todo: 8
Done: 22, Todo: 7
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Fix formatting in the count summary.

The counts are correctly updated to reflect the Fisher Transform completion. However, there's a minor formatting issue.

-Done: 22, Todo: 7
+Done: 22, To-do: 7
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Done: 22, Todo: 7
Done: 22, To-do: 7
🧰 Tools
🪛 LanguageTool

[grammar] ~2-~2: It appears that a hyphen is missing in the noun “To-do” (= task) or did you mean the verb “to do”?
Context: # Oscillators indicators Done: 22, Todo: 7 ✔️ AC - Acceleration Oscillator ✔️ ...

(TO_DO_HYPHEN)


✔️ AC - Acceleration Oscillator
✔️ AO - Awesome Oscillator
Expand All @@ -15,7 +15,7 @@ Done: 21, Todo: 8
CTI - Ehler's Correlation Trend Indicator
✔️ DOSC - Derivative Oscillator
EFI - Elder Ray's Force Index
FISHER - Fisher Transform
✔️ FISHER - Fisher Transform
FOSC - Forecast Oscillator
*GATOR - Williams Alliator Oscillator (Upper Jaw, Lower Jaw, Teeth)
*KDJ - KDJ Indicator (K, D, J lines)
Expand Down
Loading