Repository navigation
Fix Windows training: UTF-8 logs, direct plot spawns, safe checkpoint replace - #32
Open
TomTheHand wants to merge 1 commit into
Open
TomTheHand wants to merge 1 commit into
TomTheHand wants to merge 1 commit into
Conversation
… replace Three Windows-runtime failures, each hit on a live run: - Log writers used the locale default (cp1252 here); a raw-byte sample with an unencodable character raised UnicodeEncodeError at sample time and killed training. All four writers are now explicit UTF-8, with errors=replace on the sample/session pair. - refresh_plots spawned a bash -c chain; Windows resolves bash to WSL's bash.exe, which cannot run D:/ paths, and stderr was devnull'd - so every plot refresh failed silently. Spawn each tool directly via sys.executable, track all children (a later spawn failing no longer loses the earlier PIDs), and keep the skip-if-still-running guard. - Checkpoint saves publish with os.replace, which raises WinError 5 if any reader holds the destination - our own unwaited plot children do exactly that. Bounded retry with exponential backoff keeps publication atomic; persistent failures still raise. POSIX behavior is unchanged. Regression tests cover unicode sample writes, plot argv/process state, retry exhaustion, and a real Windows reader-lock race; untouched upstream fails them.
Author
|
If it's not obvious enough, the above was written by AI 😅 Sorry. But as with my other PR, a few small changes to file handling get mini-AGI running on Windows without hurting anything else. |
GroveMinting
pushed a commit
to GroveMinting/mini-AGI
that referenced
this pull request
Oct 22, 2026
… replace Carry upstream PR volotat#32 into the fork. The store conflict is resolved by keeping the fork manifest writer and prune-after-publish ordering, while routing every publish through the new retrying _replace helper; _savez keeps temp cleanup on failure. The Windows sample test now exercises the fork implementation.
GroveMinting
added a commit
to GroveMinting/mini-AGI
that referenced
this pull request
Oct 22, 2026
Upstream has not been merging community PRs. Record the fork-first policy, close the stale upstream handoff wording, and document the carried Windows runtime fixes plus the pending aa9d53f port.
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.
Summary
Three Windows-runtime failures, each hit on a live training run on this machine:
UnicodeEncodeErrorat sample time and killed the run mid-training. All four log writers are now explicit UTF-8, witherrors="replace"on the session banner and sample log, where unencodable raw bytes are expected rather than exceptional.refresh_plots()spawned abash -c "... && ..."chain. On Windows,CreateProcessmay resolvebashto WSL'sbash.exe, which cannot executeD:/...paths - and stderr went to devnull, so every plot refresh failed silently. The tools are now spawned directly viasys.executablewith argv lists. All children are tracked (a later spawn failing no longer loses earlier PIDs), and the skip-if-still-running guard is preserved; the two plots render concurrently, which is safe because they write distinct outputs.os.replace, which raisesPermissionError(WinError 5) on Windows when any reader holds the destination - our own unwaited plot children openmanifest.jsonand do exactly that. The window is milliseconds wide: one save survived, the next died. A bounded retry (8 attempts, exponential backoff, ~6.4 s worst case) keeps publication atomic; persistent failures still raise, and non-permission errors propagate immediately.POSIX behavior is unchanged throughout.
Tests
tests/test_windows_runtime.py(10 tests, no torch import, no training, no production files): unicode sample-log writes, plot argv/process-state handling, retry exhaustion and immediate propagation of non-permission errors, plus a real Windows reader-lock race executed in a temp directory.Untouched upstream sources reproduce these failures (5 failures + 3 errors, including the actual cp1252
UnicodeEncodeErrorand actualWinError 5); this branch passes all 10.