Skip to content

Fix percentChange formula and Sharpe ratio calculation #82

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.

Sign up for GitHub

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

Open
sahilsk11 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix percentChange formula and Sharpe ratio calculation #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion internal/calculator/factor_expression.service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ func (h factorMetricsHandler) PricePercentChange(pr *data.PriceCache, symbol str
}

func percentChange(end, start float64) float64 {
return ((end - start) / end) * 100
return ((end - start) / start) * 100
}

func (h factorMetricsHandler) AnnualizedStdevOfDailyReturns(ctx context.Context, pr *data.PriceCache, symbol string, start, end time.Time) (float64, error) {
Expand Down
2 changes: 1 addition & 1 deletion internal/calculator/metrics.service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func CalculateMetrics(backtestResults []BacktestResult, relevantTradingDays []ti
numYears := numHours / (365 * 24)
annualizedReturn := math.Pow((endValue/startValue), 1/numYears) - 1

sharpeRatio := annualizedReturn / stdev
sharpeRatio := annualizedReturn / annualizedStdev
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Guard zero-volatility cases before Sharpe division.

Line 57 can emit Inf/NaN when annualizedStdev == 0, and this value flows to persistence/API consumers.

💡 Proposed fix 
-	sharpeRatio := annualizedReturn / annualizedStdev
+	sharpeRatio := 0.0
+	if annualizedStdev != 0 {
+		sharpeRatio = annualizedReturn / annualizedStdev
+	}
📝 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
sharpeRatio := annualizedReturn / annualizedStdev
sharpeRatio := 0.0
if annualizedStdev != 0 {
sharpeRatio = annualizedReturn / annualizedStdev
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/calculator/metrics.service.go` at line 57, Guard the Sharpe division
by checking annualizedStdev before computing sharpeRatio: replace the direct
division sharpeRatio := annualizedReturn / annualizedStdev with a safe branch
that treats zero (or near-zero) volatility as a special case (e.g. if
math.Abs(annualizedStdev) < 1e-12 then set sharpeRatio = 0.0) else compute
sharpeRatio = annualizedReturn / annualizedStdev; use math.Abs and a small
epsilon to avoid floating-point edge cases and update any callers/serializers to
expect the chosen fallback value.


return &CalculateMetricsResult{
AnnualizedStdev: annualizedStdev,
Expand Down
55 changes: 55 additions & 0 deletions internal/calculator/percent_change_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package calculator

import (
"math"
"testing"
)

func Test_percentChange(t *testing.T) {
tests := []struct {
name string
end float64
start float64
expected float64
}{
{
name: "simple increase",
end: 110,
start: 100,
expected: 10.0,
},
{
name: "simple decrease",
end: 90,
start: 100,
expected: -10.0,
},
{
name: "no change",
end: 100,
start: 100,
expected: 0.0,
},
{
name: "double",
end: 200,
start: 100,
expected: 100.0,
},
{
name: "50 percent drop",
end: 50,
start: 100,
expected: -50.0,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := percentChange(tt.end, tt.start)
if math.Abs(got-tt.expected) > 0.0001 {
t.Errorf("percentChange(%v, %v) = %v, want %v", tt.end, tt.start, got, tt.expected)
}
})
}
}
49 changes: 49 additions & 0 deletions internal/data/percent_change_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package data

import (
"math"
"testing"
)

func Test_percentChange(t *testing.T) {
tests := []struct {
name string
end float64
start float64
expected float64
}{
{
name: "simple increase",
end: 110,
start: 100,
expected: 10.0,
},
{
name: "simple decrease",
end: 90,
start: 100,
expected: -10.0,
},
{
name: "no change",
end: 100,
start: 100,
expected: 0.0,
},
{
name: "double",
end: 200,
start: 100,
expected: 100.0,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := percentChange(tt.end, tt.start)
if math.Abs(got-tt.expected) > 0.0001 {
t.Errorf("percentChange(%v, %v) = %v, want %v", tt.end, tt.start, got, tt.expected)
}
})
}
}
2 changes: 1 addition & 1 deletion internal/data/price.service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (pr *PriceCache) Get(symbol string, date time.Time) (float64, error) {
}

func percentChange(end, start float64) float64 {
return ((end - start) / end) * 100
return ((end - start) / start) * 100
}
Comment on lines 99 to 101
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add a zero-denominator guard in percentChange before computing returns.

Line 100 divides by start unconditionally. If start == 0, this yields a non-finite result and can corrupt the stdev cache computation path.

💡 Proposed fix 
 func percentChange(end, start float64) float64 {
+	if start == 0 {
+		return math.NaN()
+	}
 	return ((end - start) / start) * 100
 }
📝 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
func percentChange(end, start float64) float64 {
return ((end - start) / end) * 100
return ((end - start) / start) * 100
}
func percentChange(end, start float64) float64 {
if start == 0 {
return math.NaN()
}
return ((end - start) / start) * 100
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/data/price.service.go` around lines 99 - 101, The percentChange
function divides by start unconditionally; add a guard in percentChange to
handle start == 0 (e.g., return 0.0 or another defined sentinel) before
performing ((end - start) / start) * 100 to avoid producing non-finite values
that break downstream stdev cache logic; update percentChange to check start ==
0 and return the chosen safe value, and ensure callers that expect non-finite
values still behave correctly.


func stdevsFromPriceMap(minMaxMap map[string]*minMax, priceCache map[string]map[string]float64, stdevInputs []LoadStdevCacheInput, tradingDays []time.Time) (*stdevCache, error) {
Expand Down
Loading