fix: make the bandit reward's ARR term live and warn on missing qlib metrics - #1479
Open
michaelraziel-bit wants to merge 4 commits into
Open
fix: make the bandit reward's ARR term live and warn on missing qlib metrics#1479michaelraziel-bit wants to merge 4 commits into
michaelraziel-bit wants to merge 4 commits into
Conversation
The key "1day.excess_return_with_cost.annualized_return " (with a trailing space) never matches anything Qlib logs, so result.get() always returned the 0.0 default. That left the 0.25-weight ARR term of the bandit reward permanently dead, and the derived return/drawdown feature with it. Refs microsoft#1451 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Rf4jRK6VVz9qZQ9fcDErw
Collect the Qlib metric key strings in rdagent/scenarios/qlib/metrics.py and import them from both bandit.py and feedback.py, so the two places can no longer disagree on spelling. In extract_metrics_from_experiment, replace the silent result.get(key, 0.0) defaults with a helper that logs a warning naming the missing key and the keys that were present, then returns the default. The loop still degrades rather than dies on a partial Qlib result, but a mistyped or missing key is now visible in the log. The outer except Exception is narrowed to the one case it was protecting against (experiment.result is None after a failed run), so programming errors are no longer swallowed. The default for max_drawdown becomes 0.0 instead of 1.0. Qlib reports drawdown as a number <= 0, so a default of 1.0 would have put -1.0 in the feature vector and flipped the sign of the return/drawdown ratio whenever the key was missing. Refs microsoft#1451 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Rf4jRK6VVz9qZQ9fcDErw
The eighth bandit feature is annualized return divided by the absolute max drawdown. That is a Calmar-type ratio, not a Sharpe ratio: it has no volatility term and is a monotone function of the ARR feature already in slot 4. The formula is unchanged; only the field name now says what it computes. Metrics is only constructed inside bandit.py, so no caller is affected. Refs microsoft#1451 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Rf4jRK6VVz9qZQ9fcDErw
Feed extract_metrics_from_experiment a Series carrying the exact key names Qlib logs, and check that every field is read, that a missing key produces a warning naming it, that a missing drawdown stays neutral, and that changing ARR actually moves EnvController.reward. The first test would have failed on the trailing-space key. Refs microsoft#1451 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Rf4jRK6VVz9qZQ9fcDErw
Author
|
@microsoft-github-policy-service agree |
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 #1451
What was wrong
extract_metrics_from_experimentinrdagent/scenarios/qlib/proposal/bandit.pylooked up the annualized return with a trailing space in the key ("...annualized_return "). Qlib never logs that key,.getsilently returned the0.0default, and the ARR feature was always zero. The eighth feature (arr / -mdd) is derived from it, so it was always zero too. Between them that is 0.45 of the reward weight, so every defaultrdagent fin_quantrun has been steering the factor/model bandit on IC, ICIR, Rank IC, Rank ICIR, IR and drawdown only.Nothing noticed because every lookup defaulted to
0.0inside a bareexcept Exception, so a mistyped key, a missing key and a genuinely zero metric were indistinguishable.What this PR does
Four commits, each droppable on its own:
fix:remove the trailing space. This is the whole bug fix.refactor:move the Qlib metric key strings intordagent/scenarios/qlib/metrics.pyand import them from bothbandit.pyandfeedback.py(IMPORTANT_METRICSis now built from the constants), so the two places cannot disagree on spelling again. Replace the silent defaults with a helper that logs a warning naming the missing key and the keys that were present, then still returns the default so a partial Qlib result degrades instead of crashing the loop. Narrow the outerexcept Exceptionto the case it was guarding (experiment.result is Noneafter a failed run). Change themax_drawdowndefault from1.0to0.0: Qlib reports drawdown as a number<= 0, so1.0would have put-1.0in the feature vector and flipped the sign of the ratio whenever the key was missing.refactor:rename thesharpefield tocalmar. The formula (arr / -mdd) is unchanged; the name now says what it computes.Metricsis only constructed insidebandit.py, so nothing else is touched. This commit is optional. If you would rather keep the name, or replace the feature with a true Sharpe from Qlib'sinformation_ratioormean / std(the current feature is a monotone function of ARR, which is already slot 4, and carries no volatility information), say so and I will adjust.test:test/qlib/test_bandit_metrics.pyfeeds the extractor a Series with the exact key names Qlib logs and checks that every field is read, that a missing key produces a warning naming it, that a missing drawdown stays neutral, and that changing ARR movesEnvController.reward. The first test fails on the old key. All tests are markedofflineso they run in CI.Reward vector semantics are otherwise unchanged: same eight slots, same weights, same
-mddsign convention.log/ui/app.pystill has its own copy of the key list; left alone here to keep the diff focused, happy to switch it to the constants in a follow-up.Checks
pytest test/qlib -q: 12 passedblack --check -l 120 .andisort --check .as in the Makefile: clean馃 Generated with Claude Code
https://claude.ai/code/session_011Rf4jRK6VVz9qZQ9fcDErw
馃摎 Documentation preview 馃摎: https://RDAgent--1479.org.readthedocs.build/en/1479/