Fix inf score from division by zero in absolute price scaling#229
Open
sniper-noob wants to merge 3 commits intomode-network:mainfrom
Open
Fix inf score from division by zero in absolute price scaling#229sniper-noob wants to merge 3 commits intomode-network:mainfrom
sniper-noob wants to merge 3 commits intomode-network:mainfrom
Conversation
Problem:
When real_price_path[-1] is 0 (or NaN), the absolute price scaling at
crps_calculation.py line 99 divides by zero, producing an inf score.
The safety check at reward.py uses np.isnan(score), which does NOT
catch inf (np.isnan(inf) == False). The inf score passes through to
compute_prompt_scores where it poisons np.percentile(scores, 90),
making percentile90 = inf. This destroys the capping mechanism and
concentrates nearly all reward weight on a single miner.
Example:
10 miners with scores [50, 60, 70, 80, 90, 100, 110, 120, inf, 150]
percentile90 = inf (should be ~135)
np.minimum(scores, inf) = scores (no capping applied)
After softmax: miner 0 gets 86.5% of all rewards
Everyone else is suppressed, regardless of their actual quality
Fix:
crps_calculation.py line 97-100:
Guard absolute price scaling — skip block if last_price is 0 or
not finite, instead of dividing by it
reward.py lines 108, 422:
np.isnan(score) -> not np.isfinite(score)
Catches both NaN AND inf, returning -1 as intended
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
23f9eba to
daac5a0
Compare
Flake8 C901: calculate_crps_for_miner exceeded max-complexity=10. Extracted block CRPS loop into helper, which also contains the div-by-zero guard for absolute price scaling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
|
Hi @sniper-noob thanks for the contribution, can you add type hint please? |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a reward-poisoning failure mode where CRPS absolute-price scaling could divide by a zero/NaN final real price, producing inf scores that bypassed NaN-only checks and broke percentile-based capping in reward computation.
Changes:
- Add a guard in CRPS absolute-price scaling to skip scaling when the final real price is
0or non-finite. - Treat non-finite CRPS scores (
NaNandinf) as invalid in the reward worker (np.isfinite). - Add regression tests covering absolute-price last-real
0/NaNcases and mixed-interval scoring.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
synth/validator/crps_calculation.py |
Extracts per-block CRPS computation and adds a non-finite/zero last-price guard for absolute-price scaling. |
synth/validator/reward.py |
Hardens worker validation to reject non-finite CRPS scores (handles inf as well as NaN). |
tests/test_calculate_crps.py |
Adds regression tests to ensure CRPS scores stay finite when the last real price is 0/NaN. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
synth/validator/crps_calculation.py
Outdated
Comment on lines
+13
to
+15
| simulated_changes, real_changes, data_blocks, absolute_price, last_price | ||
| ): | ||
| """Compute CRPS for observed blocks, returns (total, details list).""" |
Comment on lines
+532
to
+548
| # 10 normal scores + 1 that would be inf without fix | ||
| scores = np.array([50, 60, 70, 80, 90, 100, 110, 120, 130, 150]) | ||
| prompt_scores, p90, lowest = compute_prompt_scores(scores) | ||
| self.assertTrue(np.all(np.isfinite(prompt_scores))) | ||
| self.assertTrue(np.isfinite(p90)) | ||
|
|
||
| # With inf injected (simulating old bug) | ||
| scores_with_inf = np.array( | ||
| [50, 60, 70, 80, 90, 100, 110, 120, np.inf, 150] | ||
| ) | ||
| ps_inf, p90_inf, _ = compute_prompt_scores(scores_with_inf) | ||
| # p90 with inf is itself inf, destroying capping | ||
| self.assertFalse( | ||
| np.isfinite(p90_inf), | ||
| "This proves inf poisons percentile90", | ||
| ) | ||
|
|
- Add full type annotations to _compute_block_crps function signature - Add type hints to calculate_price_changes_over_intervals boolean parameters - Improves code clarity per owner request
Contributor
Author
@Thykof done:) |
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:
When real_price_path[-1] is 0 (or NaN), the absolute price scaling at crps_calculation.py line 99 divides by zero, producing an inf score. The safety check at reward.py uses np.isnan(score), which does NOT catch inf (np.isnan(inf) == False). The inf score passes through to compute_prompt_scores where it poisons np.percentile(scores, 90), making percentile90 = inf. This destroys the capping mechanism and concentrates nearly all reward weight on a single miner.
Example:
10 miners with scores [50, 60, 70, 80, 90, 100, 110, 120, inf, 150]
percentile90 = inf (should be ~135)
np.minimum(scores, inf) = scores (no capping applied)
After softmax: miner 0 gets 86.5% of all rewards
Everyone else is suppressed, regardless of their actual quality
Fix:
crps_calculation.py line 97-100:
Guard absolute price scaling — skip block if last_price is 0 or
not finite, instead of dividing by it
reward.py lines 108, 422:
np.isnan(score) -> not np.isfinite(score)
Catches both NaN AND inf, returning -1 as intended