fix(forecasting): resolve regressor source collisions deterministically#2354
Open
Flix6x wants to merge 1 commit into
Open
fix(forecasting): resolve regressor source collisions deterministically#2354Flix6x wants to merge 1 commit into
Flix6x wants to merge 1 commit into
Conversation
When several selected sources record a belief about the same event at the same belief time, the pipeline used to keep whichever row it happened to see first, making forecasts dependent on row order. Now the collision is resolved deterministically: the order of an explicit sources list decides precedence (first listed wins), and otherwise the source ranked highest by source_priorities wins (latest version, then highest ID). Deduplicating right after loading also prevents duplicated (event_start, belief_time) keys from multiplying rows through the outer join over regressors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WF4PYUDxbBQyPf37PAUKBC Signed-off-by: F.N. Claessen <felix@seita.nl>
Documentation build overview
7 files changed ·
|
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
1 participant
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.
Description
Follow-up to #2347, addressing the source-collision question raised in this comment thread: when several selected sources record a belief about the same event at the same belief time, the pipeline kept whichever row it happened to see first (
idxmaxreturns the first occurrence of the max belief time), so forecasts depended on row order.This PR makes the resolution deterministic:
sourceslist order decides precedence — the first listed source wins on collisions, givingsources: [1212, 868]the natural reading "prefer 1212, fall back to 868" (the fallback part already works via the outer join for events the preferred source doesn't cover).source_prioritieshelper wins (latest source version, then highest ID) — the same rule the platform uses elsewhere.search_beliefs, while thesourcecolumn is still available. This also fixes a subtle correctness issue: duplicated(event_start, belief_time)keys used to multiply rows through the chained outer join over regressors inload_data_all_beliefs.sourcesfield description (schema + OpenAPI spec) and added a changelog entry.No warning is logged on collisions. Since the rule is now deterministic and documented, a warning adds little — and for API-created jobs we have no good channel to surface it, so it would only add log volume on hosts.
@BelhsanHmida please review — and if you approve, feel free to merge this into your branch so it lands as part of #2347.
How to test
Full
test_forecasting_pipeline.py+data/schemas/tests/test_forecasting.pypass locally (83 passed).Related Items
Follow-up to #2347. Note for the remaining canonicalization item listed under Further Improvements there: with this change the order of
sourcesis semantically significant, so it should not be sorted when generating forecast data-source identities (the other filter lists are still fair game).Sign-off
🤖 Generated with Claude Code
https://claude.ai/code/session_01WF4PYUDxbBQyPf37PAUKBC