Repository navigation
Skip duplicate JSON conversion for prepared checkpoints after #11 - #12
Conversation
D-sudoasd
left a comment
There was a problem hiding this comment.
Independent review against main a911c2e. I found this replacement already in progress while comparing #10/#11, so I did not create another implementation branch, alter an existing workspace, or repeat the reported focused validation.
The retained scope is appropriate: #11's exact-built-in list/dict specialization and benchmark remain intact; #10's independent contribution is the prepared-payload writer. Frame/run metadata normalization, retained partial-resume metadata, and overflowed array-summary min/max are necessary companions to bypassing the final generic conversion. The synchronous write/flush/fsync/close/replace sequence, public-mapping conversion, full-history rebuild, and forward-only exact source/builder gates remain present. The expanded storage tests exercise every accepted predecessor pair and both full/partial native restoration, and abrupt-exit tests now cover both public and prepared writes. I have reviewed this code and the published validation record; I have not independently rerun the 170-test command.
CI blocker found: Windows / Python 3.12 failed before pytest. Run 38854235242, job 116822062616, failed at Check dependencies (python -m pip check), not at the checkpoint tests. The job log explicitly reports:
pipx 1.17.12 has requirement filelock>=4.0.9, but you have filelock 3.32.4.
The install log shows runner-preinstalled filelock 4.0.12 being replaced by the repository's existing constraints/validation-py311-313.txt pin, filelock==3.32.4. This is a collision with the hosted runner's preinstalled pipx in the shared interpreter. The Test and smoke steps in that matrix job were skipped, so it cannot be counted as cross-platform validation.
Please keep follow-up on this integration path and isolate the test installation in a fresh virtual environment without system-site-packages, with installation, pip check, pytest, and CLI smoke consistently using that environment. Preserve the dependency pins and the dependency check; do not relax scientific/recovery identities to make CI pass. Do not merge until the affected matrix validation and the normal remaining checks complete successfully. I have not launched a competing CI repair or rerun.
Performance interpretation remains limited to the recorded synthetic cases: the single continuous outer-timed pair improved, but the partial-resume total did not. The generic-conversion reduction alone does not establish a partial-resume speedup, and none of these numbers establish a 121-frame production speedup. Closing #10 with its branch preserved and directing further work to #12 is the correct coordination outcome.
|
Taking the user-requested final closeout on this existing PR: fix the hosted-runner dependency collision, complete normal cross-platform CI, then merge only the verified head and remove superseded/merged branches after checking their tips. Current head is still 1460a45; no newer code/discussion was found. The connected workspace API remains unavailable, so no remote working directory is being changed. The CI fix will use an isolated .venv-project so installation, pip check, pytest, CLI smoke and the existing Windows desktop launcher resolve the same environment. Dependency pins and scientific/recovery code will remain unchanged for this fix. |
|
Final closeout completed at user request. CI isolation fix 5f76bc8 passed the full six-job Linux/Windows Python 3.11-3.13 matrix, including actual pytest, dependency checks, CLI smoke and all three Windows launcher checks; lint and wheel smoke also passed. CI run: https://github.com/D-sudoasd/WingSAXS/actions/runs/38855207424 . PR merged as 96c253e; main's complete Git tree was checked against the verified head and is identical. The merged #11/#12 branches, superseded #10 branch and this temporary maintenance branch were removed together with exact-SHA leases. No temporary workflow was merged into main and no production fitting or performance rerun was performed. Existing synthetic-performance limits still apply. |
Final status — 2026-10-16
Merged into main as
96c253e65adb6b58ecab9127dc6784d0d7e8409f. The complete main Git tree matches the verified head5f76bc89b89a2586ea9d14801b94be4821a16802.The Windows/Python 3.12 hosted-runner dependency collision was fixed in
5f76bc8by installing and running the matrix in an isolated.venv-project, also used by the existing Windows desktop launcher. Dependency pins,pip check, scientific code and recovery identity checks were not relaxed.CI run 38855207424 passed all eight jobs: lint, all six Linux/Windows Python 3.11–3.13 matrix jobs, and wheel smoke. Actual pytest, dependency checks and CLI smoke passed in every matrix job; all three Windows desktop-launcher checks passed.
At the user's explicit request, the merged #11/#12 branches, the superseded #10 branch and the temporary maintenance branch were deleted with exact-SHA leases after checking the merged file tree and PR states. Remote branches now contain only main, with no open PRs. #10 remains closed and unmerged; its original commit and discussion remain available. The temporary maintenance workflow was not merged into main. No existing Agent working directory was modified.
Implementation
Before this change, checkpoint snapshots on #11 main still traversed scientific history a second time in the writer. This PR preserves #11's scalar-copy specialization and benchmark, and retains only #10's independent prepared-payload serialization change. Public mappings retain generic JSON conversion; internal frame/run metadata is normalized at construction, including retained partial-resume records and overflowed array summaries.
Based on
a911c2e9946652fe742e4dd054ebf61080ff3ee5; supersedes #10. Exact LF/CRLF predecessor source pairs from #11/#10 are added without relaxing other producer, dependency, analyzer, input/config or native-array gates. Full histories, float precision, commit frequency and synchronous durability remain unchanged.Earlier focused validation
170 focused tests passed, 3 Win32-only metadata checks skipped on Linux; default Ruff rules and diff checks passed. Tests include every accepted source pair in full/partial native recovery, NPZ/provenance preservation, mutable history, prepared/public equivalence, failure cleanup and real process exits after fsync/replace. Independent fixed-clock main/candidate subprocesses matched all four checkpoint generations by nested field/type/float.hex except verified actual producer identity.
Synthetic performance observations
Single paired 24-frame synthetic outer-timed replay: 15.10987 -> 8.96429 seconds, 25 synchronous commits each, strict readback passed. Stage-timed partial replay: 6.25683 -> 6.59625 seconds despite generic conversion decreasing 1.70838 -> 0.00783 seconds; no general partial-resume speedup is claimed. No real 121-frame production measurements or scientific fitting were performed. These observations are not combined with previous PR percentages, and no performance or production-fitting replay was repeated during final closeout.
Details, comparison decisions and reproduction:
docs/validation/checkpoint_prepared_payload.md. The original implementation used an isolated clone; final CI repair and closeout used GitHub operations and an isolated Actions runner. The connected workspace API was unavailable and no existing work environment was changed.