diff --git a/code/saga.py b/code/saga.py index 88d4467..718bba3 100644 --- a/code/saga.py +++ b/code/saga.py @@ -122,6 +122,11 @@ def __init__(self, mu, sigma, list_samples, tau=14, chi2_bucket=10, pmin=0.001): "over Z), but received non-integer values. For continuous " "multivariate data, MultivariateSamples skips the " "per-coordinate discrete channel automatically.") + # Keep the samples in their original order (by reference -- do not + # mutate them afterwards). The sequence tests of the extended battery + # (Ljung-Box, runs, block homogeneity) measure ordering, so they cannot + # be run on a histogram-reconstructed sample list, which is sorted. + self.samples = list_samples self.histogram = dict() self.outlier = 0 # Initialize histogram @@ -152,11 +157,17 @@ def __init__(self, mu, sigma, list_samples, tau=14, chi2_bucket=10, pmin=0.001): def run_extended_battery(self, samples=None, mc_B=1000): - """Run the extended test battery (Phase 3 tests).""" + """ + Run the extended test battery (Phase 3+4 tests). + + `samples` defaults to the ordered samples this object was built from. + It must never be reconstructed from the histogram: that yields a sorted + sequence, and the sequence tests (Ljung-Box, runs, block homogeneity) + would then reject any correct sampler with p ~ 0. + """ from univariate_tests import run_extended_battery if samples is None: - samples = [z for z in self.histogram - for _ in range(self.histogram[z])] + samples = self.samples self._extended = run_extended_battery( self.exp_mu, self.exp_sigma, samples, tau=self.tau, alpha=self.pmin, mc_B=mc_B, diff --git a/code/tests/test_univariate.py b/code/tests/test_univariate.py index 6c77a4d..3c2a110 100644 --- a/code/tests/test_univariate.py +++ b/code/tests/test_univariate.py @@ -49,6 +49,23 @@ def test_extended_battery_passes(self, good_univariate_vector): if isinstance(r, dict) and not r.get("passes", True)) ) + def test_extended_battery_default_samples(self, good_univariate_vector): + """The default (no `samples=`) path must use the ordered samples. + + It once rebuilt them from the histogram, i.e. sorted, which made the + sequence tests reject every correct sampler at p ~ 0. + """ + v = good_univariate_vector + uv = UnivariateSamples(v["params"]["mu"], v["params"]["sigma"], v["samples"]) + ext = uv.run_extended_battery(mc_B=200) # no samples= argument + for name in ("ljung_box", "runs_test", "block_homogeneity"): + assert ext[name]["passes"], ( + f"Good vector {v['label']}: {name} rejects on the default " + f"path (p={ext[name]['pvalue']:.6f}) -- samples were " + f"probably reordered") + assert ext["all_pass"] + assert uv.is_valid_extended + class TestBadVectors: """Flawed distributions must be detected by chi-square or extended battery."""