Skip to content

Commit 655db4f

Browse files
committed
Fix test regressions and refactor NoncentralT.ZBrent to use Brent.Solve
- Revert RSR denominator from N-1 back to N (ensures RSR=1.0 for constant-at-mean predictions); fix misleading comment - Fix assertion argument order in Test_InverseChiSquared, Test_TimeSeries, Test_kNN, and Test_MCMCDiagnostics - Replace 120-line private ZBrent method in NoncentralT with a single call to Brent.Solve, using Math.Min/Max for bracket ordering
1 parent cb67f3f commit 655db4f

6 files changed

Lines changed: 19 additions & 140 deletions

File tree

Numerics/Data/Statistics/GoodnessOfFit.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -804,8 +804,8 @@ public static double PBIAS(IList<double> observedValues, IList<double> modeledVa
804804
/// Lower RSR values indicate better model performance.
805805
/// </para>
806806
/// <para>
807-
/// <b>Note:</b> This method uses sample standard deviation (N-1 denominator) for the observed values,
808-
/// which is consistent with the R hydroGOF package implementation.
807+
/// <b>Note:</b> This method uses the population standard deviation (N denominator) for both RMSE and
808+
/// observed variability, ensuring RSR = 1.0 when predictions equal the observed mean.
809809
/// </para>
810810
/// </remarks>
811811
public static double RSR(IList<double> observedValues, IList<double> modeledValues)
@@ -828,13 +828,13 @@ public static double RSR(IList<double> observedValues, IList<double> modeledValu
828828
double rmse = Math.Sqrt(sse / n);
829829
double obsMean = sumObs / n;
830830

831-
// Second pass to compute sample standard deviation (using N-1)
831+
// Second pass to compute standard deviation
832832
double variance = 0d;
833833
for (int i = 0; i < n; i++)
834834
{
835835
variance += Tools.Sqr(observedValues[i] - obsMean);
836836
}
837-
double stdDev = Math.Sqrt(variance / (n - 1));
837+
double stdDev = Math.Sqrt(variance / n);
838838

839839
return rmse / stdDev;
840840
}

Numerics/Distributions/Univariate/NoncentralT.cs

Lines changed: 2 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
using System;
3232
using System.Collections.Generic;
3333
using Numerics.Mathematics.Optimization;
34+
using Numerics.Mathematics.RootFinding;
3435
using Numerics.Mathematics.SpecialFunctions;
3536

3637
namespace Numerics.Distributions
@@ -566,7 +567,7 @@ private double NCT_INV(double p, double df, double delta)
566567
iter = iter + 1;
567568
}
568569
// Solve for T using Brent
569-
double ANS = ZBrent(t0, t1, y0, y1, xtol, df, delta, p);
570+
double ANS = Brent.Solve(x => NCTDist(x, df, delta) - p, Math.Min(t0, t1), Math.Max(t0, t1), xtol, reportFailure: false);
570571
return ANS;
571572
}
572573

@@ -597,128 +598,6 @@ private double NCTInv0(double P, double N, double D)
597598
}
598599
}
599600

600-
private double ZBrent(double X1, double X2, double y1, double y2, double Tol, double N, double Dnc, double Perc)
601-
{
602-
double ZBrentRet = default;
603-
//
604-
// Finds the zero of NCTDist(x, n, d) - p given that [x1, x2] brackets the zero and
605-
// Y1 = value at X1, Y2 = value at X2.
606-
//
607-
// Translated from Numerical Recipes (1986).
608-
// William A. Huber, 24 March 2001.
609-
//
610-
double a;
611-
double b;
612-
var c = default(double);
613-
double fc;
614-
var D = default(double);
615-
var e = default(double);
616-
double tol1;
617-
double xm;
618-
double s;
619-
double P;
620-
double q;
621-
double r;
622-
double fa;
623-
double fb;
624-
int iter;
625-
const int itmax = 100;
626-
const double eps = 0.00000003d;
627-
a = X1;
628-
b = X2;
629-
fa = y1;
630-
fb = y2;
631-
if (fb * fa > 0d)
632-
{
633-
throw new ArgumentException("Brent's method failed because the root is not bracketed.");
634-
}
635-
636-
fc = fb;
637-
for (iter = 1; iter <= itmax; iter++)
638-
{
639-
if (fb * fc > 0d)
640-
{
641-
c = a;
642-
fc = fa;
643-
D = b - a;
644-
e = D;
645-
}
646-
647-
if (Math.Abs(fc) < Math.Abs(fb))
648-
{
649-
a = b;
650-
b = c;
651-
c = a;
652-
fa = fb;
653-
fb = fc;
654-
fc = fa;
655-
}
656-
657-
tol1 = 2.0d * eps * Math.Abs(b) + 0.5d * Tol;
658-
xm = 0.5d * (c - b);
659-
if (Math.Abs(xm) <= tol1 || fb == 0d)
660-
{
661-
return b;
662-
}
663-
664-
if (Math.Abs(e) >= tol1 && Math.Abs(fa) > Math.Abs(fb))
665-
{
666-
s = fb / fa;
667-
if (a == c)
668-
{
669-
P = 2.0d * xm * s;
670-
q = 1.0d - s;
671-
}
672-
else
673-
{
674-
q = fa / fc;
675-
r = fb / fc;
676-
P = s * (2.0d * xm * q * (q - r) - (b - a) * (r - 1.0d));
677-
q = (q - 1.0d) * (r - 1.0d) * (s - 1.0d);
678-
}
679-
680-
if (P > 0d)
681-
q = -q;
682-
P = Math.Abs(P);
683-
if (2.0d * P < 3.0d * xm * q - Math.Abs(tol1 * q) & 2.0d * P < Math.Abs(e * q))
684-
{
685-
e = D;
686-
D = P / q;
687-
}
688-
else
689-
{
690-
D = xm;
691-
e = D;
692-
}
693-
}
694-
else
695-
{
696-
D = xm;
697-
e = D;
698-
}
699-
700-
a = b;
701-
fa = fb;
702-
if (Math.Abs(D) > tol1)
703-
{
704-
b = b + D;
705-
}
706-
else if (xm > 0d) // b = b + SIGN(tol1, xm)
707-
{
708-
b = b + Math.Abs(tol1);
709-
}
710-
else
711-
{
712-
b = b - Math.Abs(tol1);
713-
}
714-
715-
fb = NCTDist(b, N, Dnc) - Perc;
716-
}
717-
718-
ZBrentRet = b;
719-
return ZBrentRet;
720-
}
721-
722601
/// <inheritdoc/>
723602
public override UnivariateDistributionBase Clone()
724603
{

Test_Numerics/Data/Time Series/Test_TimeSeries.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,8 +1008,8 @@ public void Test_SeasonalDecompose()
10081008
var (trend, seasonal, residual) = ts.SeasonalDecompose(period);
10091009

10101010
// Verify trend is returned
1011-
Assert.IsGreaterThan(trend.Count, 0, "Trend should have values.");
1012-
Assert.IsLessThanOrEqualTo(trend.Count, n, "Trend should not exceed original length.");
1011+
Assert.IsGreaterThan(0, trend.Count, "Trend should have values.");
1012+
Assert.IsLessThanOrEqualTo(n, trend.Count, "Trend should not exceed original length.");
10131013

10141014
// Verify seasonal has correct length
10151015
// HasCount does not accept double[] (not IEnumerable), so suppress MSTEST0037
@@ -1018,7 +1018,7 @@ public void Test_SeasonalDecompose()
10181018
#pragma warning restore MSTEST0037
10191019

10201020
// Verify residual is returned
1021-
Assert.IsGreaterThan(residual.Count, 0, "Residual should have values.");
1021+
Assert.IsGreaterThan(0, residual.Count, "Residual should have values.");
10221022

10231023
// Verify the decomposition is additive: original = trend + seasonal + residual
10241024
// This is an exact mathematical identity by construction
@@ -1043,7 +1043,7 @@ public void Test_SeasonalDecompose()
10431043
// Verify seasonal values are not all zero (they should have nonzero amplitude)
10441044
double maxSeasonal = seasonal.Max();
10451045
double minSeasonal = seasonal.Min();
1046-
Assert.IsGreaterThan(maxSeasonal - minSeasonal, 1.0,
1046+
Assert.IsGreaterThan(1.0, maxSeasonal - minSeasonal,
10471047
"Seasonal component should have nonzero amplitude for sinusoidal input.");
10481048
}
10491049

Test_Numerics/Distributions/Univariate/Test_InverseChiSquared.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,8 @@ public void Test_CDF_InverseCDF_Roundtrip()
266266
Assert.AreEqual(0.8911780189, dist.CDF(2.0), 1E-6);
267267

268268
// CDF must be non-decreasing
269-
Assert.IsLessThan(dist.CDF(0.5), dist.CDF(1.0));
270-
Assert.IsLessThan(dist.CDF(1.0), dist.CDF(2.0));
269+
Assert.IsLessThan(dist.CDF(1.0), dist.CDF(0.5));
270+
Assert.IsLessThan(dist.CDF(2.0), dist.CDF(1.0));
271271

272272
// InverseCDF values from v*sigma / scipy.stats.chi2.isf(p, v)
273273
Assert.AreEqual(0.6255012152, dist.InverseCDF(0.1), 1E-4);

Test_Numerics/Machine Learning/Supervised/Test_kNN.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,12 +190,12 @@ public void Test_GetNeighbors_MultiRow()
190190
Assert.IsNotNull(neighbors);
191191

192192
// First query (0.5, 0.5): nearest neighbors should be from cluster A (indices 0-5)
193-
Assert.IsLessThan(neighbors[0], 6, $"First query's nearest neighbor should be in cluster A, got index {neighbors[0]}");
194-
Assert.IsLessThan(neighbors[1], 6, $"First query's 2nd nearest should be in cluster A, got index {neighbors[1]}");
193+
Assert.IsLessThan(6, neighbors[0], $"First query's nearest neighbor should be in cluster A, got index {neighbors[0]}");
194+
Assert.IsLessThan(6, neighbors[1], $"First query's 2nd nearest should be in cluster A, got index {neighbors[1]}");
195195

196196
// Second query (100.5, 100.5): nearest neighbors should be from cluster B (indices 6-11)
197-
Assert.IsGreaterThanOrEqualTo(neighbors[2], 6, $"Second query's nearest neighbor should be in cluster B, got index {neighbors[2]}");
198-
Assert.IsGreaterThanOrEqualTo(neighbors[3], 6, $"Second query's 2nd nearest should be in cluster B, got index {neighbors[3]}");
197+
Assert.IsGreaterThanOrEqualTo(6, neighbors[2], $"Second query's nearest neighbor should be in cluster B, got index {neighbors[2]}");
198+
Assert.IsGreaterThanOrEqualTo(6, neighbors[3], $"Second query's 2nd nearest should be in cluster B, got index {neighbors[3]}");
199199
}
200200

201201
}

Test_Numerics/Sampling/MCMC/Test_MCMCDiagnostics.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,14 @@ public void Test_GelmanRubin_WithWarmup()
7676

7777
// Without warmup, R-hat should be high (chains have different initial distributions)
7878
var rhatNoWarmup = MCMCDiagnostics.GelmanRubin(chains, 0);
79-
Assert.IsGreaterThan(rhatNoWarmup[0], 1.1, $"R-hat without warmup should be > 1.1, got {rhatNoWarmup[0]}");
79+
Assert.IsGreaterThan(1.1, rhatNoWarmup[0], $"R-hat without warmup should be > 1.1, got {rhatNoWarmup[0]}");
8080

8181
// With warmup=50, R-hat should be close to 1.0 (converged portion only)
8282
var rhatWithWarmup = MCMCDiagnostics.GelmanRubin(chains, 50);
83-
Assert.IsLessThan(rhatWithWarmup[0], 1.1, $"R-hat with warmup=50 should be < 1.1, got {rhatWithWarmup[0]}");
83+
Assert.IsLessThan(1.1, rhatWithWarmup[0], $"R-hat with warmup=50 should be < 1.1, got {rhatWithWarmup[0]}");
8484

8585
// Warmup should improve R-hat (make it closer to 1.0)
86-
Assert.IsLessThan(rhatWithWarmup[0], rhatNoWarmup[0],
86+
Assert.IsLessThan(rhatNoWarmup[0], rhatWithWarmup[0],
8787
"R-hat with warmup should be closer to 1.0 than without warmup");
8888
}
8989

0 commit comments

Comments
 (0)