fix(qlib): stop dropping read_exp_res.py's log on failed backtest runs - #1482
Open
Pujitha Paladugu (pujitha24) wants to merge 1 commit into
Open
Pujitha Paladugu (pujitha24) wants to merge 1 commit into
Pujitha Paladugu (pujitha24) wants to merge 1 commit into
Conversation
Motivation:
QlibFBWorkspace.execute() runs two steps in the target docker/conda
environment: `qrun <config>` (captured as execute_qlib_log) and then
`python read_exp_res.py` (captured as execute_log), which extracts the
backtest results and raises when an expected artifact is missing (e.g.
`LoadObjectError: No such file or directory: '.../report_normal_1day.pkl'`).
When the resulting ret.parquet or qlib_res.csv file is missing, execute()
returned only execute_qlib_log, silently dropping execute_log - the one
log that actually contains read_exp_res.py's failure reason. That return
value is surfaced verbatim to the user/LLM feedback loop via
`raise FactorEmptyError(f"Failed to run this experiment, because {stdout}")`
in factor_runner.py (and the analogous path in model_runner.py), so the
real cause of the failure was being thrown away, leaving only a qrun log
that doesn't explain why the follow-up extraction step failed. This
matches the confusion in the linked report, where the surfaced error
gives no hint about the missing artifact.
This does not fix the underlying reason an artifact may go missing (e.g.
an environment-specific GPU/CUDA problem, as diagnosed in a comment on
the report) - it only preserves the diagnostic information needed to
tell that story instead of discarding it.
Approach:
Concatenate execute_log into both failure-path return values in
QlibFBWorkspace.execute() so read_exp_res.py's own output is preserved
alongside the qrun log.
Validation:
This sandbox has under 250MB of free disk space and could not install
this repo's Python dependencies (pandas, pydantic, docker, pytest, etc.)
to run `pytest test/qlib/test_workspace.py` directly, so that command
was not run here.
Instead, the change was validated by importing the real, unmodified
rdagent/scenarios/qlib/experiment/workspace.py directly, stubbing only
its heavy transitive dependencies (pandas, rdagent.utils.env,
rdagent.core.experiment, rdagent.log,
rdagent.components.coder.model_coder.conf) with lightweight
stdlib-only fakes, and driving QlibFBWorkspace.execute() through the
same missing-ret.parquet code path the new test exercises:
- against the pre-fix code (`git stash`), it fails with
AssertionError: LoadObjectError missing from the returned log
- against the fix, it passes: both the qrun log and read_exp_res.py's
log are present in the returned log
`python3 -m py_compile` was also run on both changed files with no
errors. The new test/qlib/test_workspace.py encodes this same
fail-before/pass-after check using this repo's normal
unittest.mock-based pytest style, but pytest itself could not be
executed in this sandbox for the reason above.
Report: microsoft#442
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Assisted-by: claude-sonnet-5 (via Claude Code)
Author
|
Just checking in — this has been rebased and green for a week; happy to make any changes that would help it move forward. |
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.
Description
QlibFBWorkspace.execute()runsqrun <config>(captured asexecute_qlib_log) and thenpython read_exp_res.py(captured asexecute_log) inside the target docker/conda environment.read_exp_res.pyis what raisesLoadObjectError: No such file or directory: '.../report_normal_1day.pkl'when an expected backtest artifact is missing.When the resulting
ret.parquetorqlib_res.csvfile doesn't exist,execute()returned onlyexecute_qlib_log, silently droppingexecute_log— the log that actually containsread_exp_res.py's failure reason. This return value is surfaced verbatim to the user/LLM feedback loop viaraise FactorEmptyError(f"Failed to run this experiment, because {stdout}")infactor_runner.py(and the analogous path inmodel_runner.py), so the real cause of the failure was thrown away.This change concatenates
execute_loginto both failure-path return values soread_exp_res.py's own output is preserved alongside the qrun log.This does not fix the underlying reason an artifact may go missing (e.g. an environment-specific GPU/CUDA problem, as diagnosed in a comment on the linked issue) — it only preserves the diagnostic information needed to explain that, instead of discarding it.
Motivation and Context
Report: #442
The reporter saw the workflow stall with a confusing downstream error (
LoadObjectErroronreport_normal_1day.pkl) and no visible root cause. A commenter traced their own case to a CUDA/GPU environment issue causing the backtest artifact to never be generated — but regardless of root cause, RD-Agent's own error-reporting path was silently discarding the one log (read_exp_res.py's output) that would show why the artifact is missing, making this class of failure needlessly hard to diagnose.How Has This Been Tested?
test/qlib/test_workspace.py, which mocks the docker/conda environment and asserts that when the backtest result file is missing, the returned log contains both theqrunlog andread_exp_res.py's own failure output (in this repo's existingunittest.mock-based pytest style, matchingtest/qlib/test_model_factor_proposal.py).pytest test/qlib/test_workspace.pyitself could not be run here. Instead, the change was validated by importing the real, unmodifiedworkspace.py, stubbing only its heavy transitive dependencies with lightweight stdlib-only fakes, and drivingQlibFBWorkspace.execute()through the same code path: it fails (AssertionError,LoadObjectErrormissing from the log) against the pre-fix code and passes against the fix.python3 -m py_compilewas also run on both changed files with no errors.main's own CI (theCIworkflow) is currently green, so no known unrelated red checks to disclose.Screenshots of Test Results (if appropriate):
Not applicable (no UI change; see "How Has This Been Tested?" above for the console-based validation performed).
Types of changes
Fixes #442
📚 Documentation preview 📚: https://RDAgent--1482.org.readthedocs.build/en/1482/