Conversation
run_command captured a model's stdout and stderr with communicate(), which blocks until the process exits, and callers discard the returned string on success. A model that ran for minutes was therefore silent throughout, and anything it logged was visible only when it failed. Read the pipe line by line instead and log each line at debug level, so a long backtest reports progress as it goes. The output is still returned in full and still embedded in the exception raised on a non-zero exit. stderr is merged into stdout so the two streams keep the order the model produced them; they were concatenated into a single string here anyway. Set PYTHONUNBUFFERED in the subprocess environment as well: Python block-buffers stdout when it is not a terminal, which would hold the output back until exit and defeat the streaming. Every runner that shells out goes through Runner._execute, so uv, conda and renv all pick this up. The docker path captures its logs separately and is unchanged.
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.
What
run_commandcaptured a model's stdout and stderr withcommunicate(), which blocks until the process exits, andExternalModel.train()/.predict()discard the returned string on success. A model that ran for minutes was silent the whole time, and anything it logged was visible only if it failed.This reads the pipe line by line and logs each line at debug level with a
[model]prefix, so a long backtest reports progress as it goes.Why it needs both halves
Streaming on this side is necessary but not sufficient: Python block-buffers stdout when it is not a terminal, so a model's log lines would still arrive in 4-8KB lumps, or all at once at exit.
PYTHONUNBUFFEREDis now set in the subprocess environment too. Either change alone leaves the output effectively unstreamed.Behaviour preserved
CommandLineExceptionraised on a non-zero exit, so failures read exactly as before.stderris merged intostdoutso the two streams keep the order the model produced them. They were concatenated into a single string here anyway, so nothing is lost; what improves is that interleaved output is no longer reordered into "all stdout, then all stderr".Scope
Every runner that shells out goes through
Runner._executeand so through this function, which covers uv, conda and renv. The docker path captures its logs separately (container.wait()thencontainer.logs()) and is unchanged; it would need the equivalentstream=Truetreatment as a follow-up.Debug level rather than info is deliberate: a 12-split backtest at
n_iter 500produces a lot of per-epoch output, which would bury chap's own logging if it were on by default.--run-config.debugopts in.Tests
test_run_command_streams_output_to_debug_logasserts the lines are logged individually and in the order produced, across both streams.test_run_command_unbuffers_subprocess_pythonassertsPYTHONUNBUFFEREDactually reaches the subprocess.make lint,make checkandmake testall pass (1462 passed, 122 skipped, 4 xfailed, 1 xpassed).