Fix numerical instability in PearsonCorrelation - #3830
Open
RangeshPandianPT wants to merge 1 commit into
Open
RangeshPandianPT wants to merge 1 commit into
RangeshPandianPT wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates ignite.metrics.regression.PearsonCorrelation to avoid catastrophic cancellation by switching from raw sum/sum-of-squares accumulation to numerically stable streaming mean/variance/covariance (Chan-style merge), and adds/updates tests to cover the regression scenario.
Changes:
- Replaced naive variance/covariance computation with stable batch statistics merged across
update()calls. - Implemented distributed aggregation via
idist.all_gather()and state merging. - Added regression/edge-case tests and updated accumulator-focused tests to reflect the new internal state.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| ignite/metrics/regression/pearson_correlation.py | Switches to stable mean/variance/covariance accumulation and adds distributed merge logic in compute(). |
| tests/ignite/metrics/regression/test_pearson_correlation.py | Adds a large-offset regression test and updates internal-accumulator assertions to match the new implementation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+95
to
+99
| mean_x_B = y_pred.mean(dtype=self._double_dtype) | ||
| mean_y_B = y.mean(dtype=self._double_dtype) | ||
|
|
||
| y_pred_d = y_pred.to(self._double_dtype) | ||
| y_d = y.to(self._double_dtype) |
Comment on lines
+126
to
129
| def compute(self) -> float: | ||
| if self._num_examples == 0: | ||
| raise NotComputableError("PearsonCorrelation must have at least one example before it can be computed.") | ||
|
|
Comment on lines
+102
to
+124
| offset = 1e8 | ||
| y_true = torch.tensor([1., 2., 3., 4., 5.], dtype=torch.float64) + offset | ||
| y_pred = torch.tensor([1.1, 2.1, 3.1, 4.1, 5.1], dtype=torch.float64) + offset | ||
|
|
||
| # Test single batch | ||
| m = PearsonCorrelation(device=available_device) | ||
| m.update((y_pred, y_true)) | ||
| assert m.compute() == pytest.approx(1.0, rel=1e-4) | ||
|
|
||
| # Test multiple update calls | ||
| # Test multiple update calls with batch size 1 | ||
| m.reset() | ||
| for yp, yt in zip(y_pred, y_true): | ||
| m.update((yp.unsqueeze(0), yt.unsqueeze(0))) | ||
|
|
||
| assert m.compute() == pytest.approx(1.0, rel=1e-4) | ||
|
|
||
| # Test different batch sizes | ||
| m.reset() | ||
| m.update((y_pred[:2], y_true[:2])) | ||
| m.update((y_pred[2:], y_true[2:])) | ||
|
|
||
| assert m.compute() == pytest.approx(1.0, rel=1e-4) |
Comment on lines
+165
to
+168
| total_mean_x += delta_x * (n_B / total_n) | ||
| total_mean_y += delta_y * (n_B / total_n) | ||
| else: | ||
| total_n = self._num_examples |
Comment on lines
+126
to
+130
| # Test zero variance edge case (constant inputs) | ||
| m.reset() | ||
| y_true_zero_var = torch.tensor([1., 1., 1., 1., 1.], dtype=torch.float64) + offset | ||
| y_pred_zero_var = torch.tensor([2., 2., 2., 2., 2.], dtype=torch.float64) + offset | ||
|
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3662
Description
Fixes numerical instability in
PearsonCorrelationcaused bycatastrophic cancellation in the variance and covariance calculations.
The previous implementation accumulated raw sums and squares, which
can become numerically unstable for inputs with large offsets.
This change uses batch-level mean, variance, and covariance statistics
combined with Chan's merge algorithm to maintain numerical stability
across multiple
update()calls and distributed processes.Changes
idist.all_gather().Testing
Ran:
pytest tests/ignite/metrics/regression/test_pearson_correlation.pyResult:
The distributed test setup error is environment-specific and does not represent a test assertion failure.
Checklist