fix: correct merger premium indexing, crisis correlation loss, and noise bound API - #3
Merged
hongjin-he merged 1 commit intoAug 3, 2026
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes three correctness defects in the financial event operator algebra and dual-noise utilities: (1) merger operator output-row indexing for deal-related shifts, (2) systemic crisis operator losing intended cross-asset price correlation, and (3) an invalid @property API on DualNoiseParams.cramer_rao_bound. It also adds regression tests to prevent reintroducing these behaviors.
Changes:
- Fix
merger_operatorto apply premium/volume/leverage/price-uncertainty shifts at the acquirer’s output row rather than hardcoded row 0. - Rework
systemic_crisis_operatornoise construction to preserve variances while introducing cross-asset correlation in the price block via a Cholesky factor. - Convert
DualNoiseParams.cramer_rao_boundfrom a property to a callable method and tighten NumPy typing/return types instate/noise.py; add regression tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
events/operators.py |
Fixes merger output indexing and rebuilds systemic crisis noise to include cross-asset price correlation via Cholesky. |
state/noise.py |
Fixes cramer_rao_bound API and improves NumPy typing/explicit float returns. |
tests/test_events.py |
Adds regression tests covering merger indexing and systemic crisis cross-asset correlation/PSD/edge cases. |
tests/test_noise.py |
Adds regression tests for callable/default/scaling/formula behavior of cramer_rao_bound. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
492
to
+494
| rho = 0.5 + 0.4 * severity | ||
| Sigma = np.zeros((n * d, n * d)) | ||
| sigma_p = severity * 0.08 # per-asset price std dev | ||
| Cov = np.eye(n * d) * sigma_p ** 2 |
Comment on lines
32
to
34
| def cramer_rao_bound(self, h: float = 1.0) -> float: | ||
| """Prediction variance lower bound for horizon h (Theorem III.1).""" | ||
| return (self.sigma_tau**2 + self.lambda_eta * self.m2_eta) * h |
Comment on lines
+629
to
+634
| # Compute acquirer's output row index | ||
| acq_out_idx = 0 | ||
| for i in range(acquirer_idx): | ||
| if i != target_idx: | ||
| acq_out_idx += 1 | ||
|
|
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.
Problem
Three bugs affect core financial event operators and the dual-noise module:
merger_operatordata corruption — the deal premium, volume spike, leverage shift, and elevated price uncertainty were hardcoded at output row 0. With a nonzero acquirer index, these shifts silently landed on an unrelated pass-through asset.systemic_crisis_operatorsilent math error — the implementation constructed cross-asset price covariance, then discarded every off-diagonal throughnp.diag(...). Crisis simulations therefore generated independent rather than correlated asset moves.DualNoiseParams.cramer_rao_boundAPI defect — the function was decorated with@propertydespite acceptingh, making the horizon inaccessible and causing explicit calls to raiseTypeError.Closes #2.
Root cause
b[0 * d + ...]instead of tracking the acquirer's row after the target is removed.np.diag(matrix)extracted the diagonal before the noise factor was rebuilt, erasing the intended price covariance.state/noise.pyalso retained an unused import and imprecise NumPy return annotations that produced editor/static-analysis errors.Implementation
acq_out_idxfrom the merger output mapping and use it for the premium, volume, leverage, and price-uncertainty entries.Sigma_w.@propertyfromcramer_rao_bound, retainingh=1.0as the default.state/noise.pyexplicit and remove the unused import.Why this approach
The patch is local to the faulty indexing, covariance construction, and API declaration. It adds no dependency and preserves the operator equation
T_w(s) = A_w s + b_w + Sigma_w epsilon. Cholesky factorization is used becauseSigma_w @ Sigma_w.Tmust reproduce the desired covariance rather than storing covariance entries directly as noise coefficients.Before and after
acquirer_idx != 0modifies output row 0. After: all deal shifts follow the acquirer into its output row.rho = 0.5 + 0.4 * severity.params.cramer_rao_bound(h=5)raisesTypeError. After: it returns(sigma_tau^2 + lambda_eta * m2_eta) * 5.Tests
Eleven regression tests cover:
Commands run:
The six warnings come from the pre-existing
build_feature_matrixNaN handling and are unrelated to this patch.Compatibility, performance, and risk
merger_operator(acquirer_idx=0, ...)is unchanged.cramer_rao_bound; the corrected method form therefore has no internal migration.O((nd)^3), but these event matrices are constructed once per event and are small in existing usage.Reviewer notes
The eleven tests were checked against the original behaviors: the primary regression assertions fail before the patch and pass after it. The final diff is limited to
events/operators.py,state/noise.py, and their two test modules.