Sortino returns 0.0 when there is no losing period, and the report paints it "Suboptimal"

investing_algorithm_framework/analysis/backtest_window_analysis.py

@@ -162,7 +162,10 @@ def analyze_backtest_windows(
         )
         sortino = (
             (mean_return * periods_per_year) / downside_vol
-            if downside_vol > 0 else 0.0
+            if downside_vol > 0
+            # No period fell below the target. Same convention as
+            # get_sortino_ratio, get_profit_factor and get_omega_ratio.
+            else (float('inf') if mean_return > 0 else 0.0)
         )
 
         rolling_max = price.cummax()

investing_algorithm_framework/services/metrics/sortino_ratio.py

@@ -54,18 +54,26 @@ def get_sortino_ratio(
             (e.g., 0.047 for 4.7%).
 
     Returns:
-        float: The Sortino Ratio.
+        float: The Sortino Ratio. Returns ``0.0`` when there is not enough
+            data, and ``float('inf')`` when no period fell below the target
+            but the excess return is positive (mirrors the division-by-zero
+            convention used by ``get_profit_factor`` and ``get_omega_ratio``).
     """
     snapshots = sorted(snapshots, key=lambda s: s.created_at)
 
     if not snapshots:
-        return float('inf')
+        return 0.0
 
     mean_daily_return = get_mean_daily_return(snapshots)
     std_downside_daily_return = get_downside_std_of_daily_returns(snapshots)
 
     if std_downside_daily_return == 0:
-        return 0.0
+        # No period fell below the target, so there is no downside risk to
+        # divide by. This mirrors the division-by-zero convention already used
+        # by get_profit_factor and get_omega_ratio: unbounded when the excess
+        # return is positive, 0.0 when there is nothing to reward.
+        excess_return = mean_daily_return * 365 - risk_free_rate
+        return float('inf') if excess_return > 0 else 0.0
 
     # Formula: Sharpe Ratio = (Mean Daily Return × Periods Per Year - Risk-Free Rate) /
     # (Standard Deviation of Daily Returns × sqrt(Periods Per Year))

tests/app/reporting/metrics/test_sortino_ratio.py

@@ -29,25 +29,25 @@ def test_no_snapshots(self):
         report.get_snapshots.return_value = []
         self.assertEqual(
             get_sortino_ratio(report.get_snapshots(), risk_free_rate=0.027),
-            float("inf")
+            0.0
         )
 
     # def test_single_snapshot(self):
     #     report = MagicMock()
     #     report.get_snapshots.return_value = [Snapshot(1000, datetime.now())]
     #     self.assertEqual(get_sortino_ratio(report.get_snapshots()), float("inf"))
 
-    # def test_all_returns_above_risk_free(self):
-    #     now = datetime.now()
-    #     snapshots = [
-    #         Snapshot(100, now),
-    #         Snapshot(110, now + timedelta(days=1)),
-    #         Snapshot(121, now + timedelta(days=2)),  # +10% twice
-    #     ]
-    #     report = MagicMock()
-    #     report.get_snapshots.return_value = snapshots
-    #     result = get_sortino_ratio(report.get_snapshots(), risk_free_rate=0.01)
-    #     self.assertEqual(result, float('inf'))
+    def test_all_returns_above_risk_free(self):
+        now = datetime.now()
+        snapshots = [
+            Snapshot(100, now),
+            Snapshot(110, now + timedelta(days=1)),
+            Snapshot(121, now + timedelta(days=2)),  # +10% twice
+        ]
+        report = MagicMock()
+        report.get_snapshots.return_value = snapshots
+        result = get_sortino_ratio(report.get_snapshots(), risk_free_rate=0.01)
+        self.assertEqual(result, float('inf'))
 
     def test_mixed_returns(self):
         now = datetime.now()

tests/services/metrics/test_sortino_ratio.py

@@ -161,8 +161,9 @@ def test_sortino_ratio_empty_snapshots(self):
         """Test with empty snapshot list."""
         result = get_sortino_ratio([], risk_free_rate=0.03)
 
-        # Should return infinity as per current implementation
-        self.assertEqual(result, float('inf'))
+        # No data is not an unbounded result. Matches get_omega_ratio, which
+        # returns 0.0 when there are no returns to measure.
+        self.assertEqual(result, 0.0)
 
     def test_sortino_ratio_single_snapshot(self):
         """Test with single snapshot."""
@@ -195,7 +196,21 @@ def test_sortino_ratio_no_downside(self):
 
         result = get_sortino_ratio(snapshots, risk_free_rate=0.03)
 
-        # Zero downside deviation -> should return 0.0
+        # There is no downside risk to divide by and the excess return is
+        # positive, so the ratio is unbounded. This is the same convention
+        # get_profit_factor and get_omega_ratio use for a zero denominator.
+        self.assertEqual(result, float('inf'))
+
+    def test_sortino_ratio_no_downside_below_risk_free(self):
+        """No losing period, but the return does not beat the risk-free rate."""
+        # +0.0001% a day is positive every day, so downside deviation is zero,
+        # but the annualised excess return is negative.
+        values = [1000 * (1.000001 ** i) for i in range(100)]
+        snapshots = self._create_snapshots(values)
+
+        result = get_sortino_ratio(snapshots, risk_free_rate=0.03)
+
+        # Unbounded would be wrong here: there is nothing to reward.
         self.assertEqual(result, 0.0)
 
     def test_sortino_ratio_constant_values(self):