diff --git a/audit/uel_multi_input_loading_audit.md b/audit/uel_multi_input_loading_audit.md index 35fbc43..c68b07f 100644 --- a/audit/uel_multi_input_loading_audit.md +++ b/audit/uel_multi_input_loading_audit.md @@ -26,5 +26,111 @@ shared by LoRA, DoRA, learned DoRA, and both text-encoder knee node families. The same change removes discarded full residual reconstruction and replaces diagonal-matrix multiplication with direct singular-value column scaling. -The asynchronous multi-input loading migration remains pending and should be -handled separately across all affected execution engines. +## Recovery contract + +This is a working-tree checkpoint, not proof that the implementation is +correct. On recovery, verify each item against the named code path and rerun +its focused test before relying on it. Passing tests prove only their +assertions. Do not infer lifecycle, release behavior, alpha mathematics, or +repository-wide coverage from an import, helper name, or checkbox. + +## Repository-wide migration progress + +- [x] Audit production `get_tensor()` and direct safetensors call sites. +- [ ] Verify the shared bounded UEL stream and atomic writer implementation. +- [ ] Verify generic and CWB merging has no hidden synchronous tensor I/O. +- [ ] Verify resize-with-base and resize use ordered UEL work units. +- [ ] Verify key utilities, metadata, passthrough, and PT conversion output. +- [ ] Verify the repository guard covers every production tensor-I/O path. +- [ ] Verify alpha normalization precedes every LoRA rank operation. +- [ ] Finish atomic-writer migration and remove obsolete imports/helpers. +- [ ] Add repository guards and complete lifecycle/failure tests. +- [ ] Run focused tests, full validation, and representative benchmarks. + +## Unverified working-tree changes + +The working tree intends `nodes/uel_io.py` to provide ordered streams, logical +multi-input work units, per-yield release, stream closure, and atomic output. +Generic recipes were changed to receive preloaded mappings. CWB, LoRA-to-model, +key utilities, metadata, and PT conversion were changed toward UEL. These are +implementation claims awaiting focused lifecycle verification. + +The intended alpha contract is `up * (alpha / rank)` before padding, alignment, +weighting, CWB, DARE/TIES, or model application. Callers were changed to replace +the original work-unit reference and omit alpha outputs. Verify every caller, +including preservation and early-return branches. The standalone normalization +script should remain untracked and unchanged; confirm this before staging. + +Evidence recorded so far: focused CWB and alpha tests passed. The first full +run reached 152 passing tests +with two failures caused only by tests reaching an implementation module's +removed `IncrementalSafetensorsWriter` import. Update those tests to import the +UEL writer directly, then rerun the full suite. A bounded search found no +production `get_tensor()` or direct safetensors imports at that point; rerun the +guard after every later edit. + +## Remaining execution checklist + +1. Model analysis was rechecked after an incorrect intermediate concern. The + caller near line 910 passes exactly one A/B tensor pair. The LoRA caller near + line 953 passes exactly two pairs for one down/up logical group, which must + coexist for pair analysis. It does not accumulate the model inventory. Both + callers release and mark their bounded unit before advancing. No change is + required unless a later test contradicts this inspection. +2. Re-run the new failure lifecycle test after explicit caller-side stream + closure changes in generic merge, CWB, and LoRA-to-model. +3. Verify extraction and resize atomic-writer conversions and remove obsolete + imports. Dtype conversion was switched to the shared atomic helper. +4. The quantization test fixture now imports the UEL writer directly. The next + full run reached 157 passing tests and one newly added lifecycle test failure; + that failure exposed missing explicit consumer-side generator closure and + code was changed afterward. Rerun the full suite. +5. The new repository guard and atomic failure test are in + `tests/test_uel_repository_contract.py`; verify they still pass. +6. Run Ruff F checks, compilation, `git diff --check`, full pytest, then a + temporary synthetic benchmark. Remove only task-created temp directories. +7. Reinspect every `async_stream()` caller for accumulation, release timing, + exceptional closure, and duplicate key occurrences. Imports and method names + are not evidence of a correct implementation. + +## Latest verification evidence + +- Full configured-environment suite: 158 tests passed. +- Focused alpha and UEL contract rerun after the full suite: 7 tests passed. +- Changed Python files compile with the configured ComfyUI interpreter. +- Ruff F checks pass for all changed implementation and test files. Unrelated + pre-existing F findings remain in untouched modules outside this change. +- `git diff --check` passes. +- The production guard finds no `.get_tensor()` calls or direct safetensors + imports under `nodes/*.py`. +- No em dash occurs in the changed audit, implementation, or focused test files. + +These results are evidence for this exact working-tree revision only. Rerun +them after any edit, merge, formatting pass, or conflict resolution. +## Compaction checkpoint: alpha normalization edge cases + +Verified on 2026-08-14 by bounded source inspection: + +- `nodes/merger.py` now records `(alpha_key, rank, down_key)` for each alpha-bearing LoRA pair and rejects alpha normalization when either factor is an unsupported low-bit tensor. Its runtime scales the loaded up factor before merge processing. +- The remaining work is to add the same explicit rejection to LoRA multi-merge and CWB LoRA guarded low-bit paths. Scalar low-bit alpha alone must remain loadable; only unsupported low-bit factor tensors make mathematical normalization impossible. +- `write_preserved_tensor` still needs failure-path lifecycle verification so every yielded tensor is marked processed and its stream is closed if writer submission raises. +- After those edits, rerun focused low-bit/alpha tests, the complete test suite in the configured ComfyUI virtual environment, changed-file compile and Ruff checks, `git diff --check`, and the production guard against direct `get_tensor()` or direct safetensors imports. +- Do not stage this work unless explicitly requested. Keep all unrelated untracked files listed by `git status` untracked. + +## Fresh validation evidence after recovery + +The following is command evidence from 2026-08-14, not a substitute for inspecting the current tree after later edits: + +- The configured ComfyUI Python completed the full suite with `160 passed in 5.52s`. +- Focused regressions for CWB and LoRA multi-merge rejection of alpha-bearing unsupported low-bit factors both passed. +- Changed implementation and test files passed Ruff `F` checks through the installed standalone Ruff executable. +- `python -m compileall -q nodes tests` completed successfully. +- `git diff --check` completed with no diff errors; Git emitted only line-ending conversion warnings. +- A scoped production search found no direct `.get_tensor(` calls and no direct safetensors imports in `nodes/*.py`. +- A scoped search found no em dash in the UEL audit, shared helpers, affected merger files, or UEL contract test. + +Implementation consequences verified by the focused source inspection and tests above: + +- Alpha-bearing LoRA factors are normalized before merge and before padding when their factor tensors are supported floating storage. +- A low-bit scalar alpha is accepted when both factors are supported; unsupported low-bit down/up factors with alpha are rejected instead of being copied into an output that falsely claims normalization. +- Preserved tensors yielded by the one-key UEL stream are marked processed in `finally`, including writer-failure paths, and the stream is closed. diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 8eb1251..fd44c99 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -1,5 +1,7 @@ # ComfyUI-ModelUtils - Complete Project History +> [Implementation Invariants](IMPLEMENTATION_INVARIANTS.md) is the authoritative contract for UEL tensor I/O, lifecycle management, atomic saving, and LoRA alpha normalization. All new node implementations and changes must follow it. This document is historical and does not override that contract. + ## Version Timeline | Version | Commits | Key Features | diff --git a/docs/IMPLEMENTATION_INVARIANTS.md b/docs/IMPLEMENTATION_INVARIANTS.md new file mode 100644 index 0000000..5786207 --- /dev/null +++ b/docs/IMPLEMENTATION_INVARIANTS.md @@ -0,0 +1,55 @@ +# Implementation Invariants + +This file is the authoritative engineering contract for model and LoRA tensor I/O and merge behavior in ComfyUI-ModelUtils. New implementations and changes to existing implementations MUST follow these rules. Historical code, audit notes, and examples do not override this contract. + +## UnifiedEfficientLoader I/O + +1. Production node code MUST use `unifiedefficientloader` for safetensors tensor I/O. Direct safetensors imports and synchronous `get_tensor()` calls are prohibited. +2. Header-only operations MUST use UEL header, key, shape, dtype, and metadata APIs without materializing tensors. +3. Tensor processing MUST use `async_stream` with bounded buffering. The repository default is `batch_size=1` and `prefetch_batches=1` unless a measured benchmark and a documented memory bound justify another value. +4. Multi-input operations MUST maintain an async cursor for each input and construct logical work units. A work unit contains only the tensors required to process one output tensor, LoRA factor pair, or other indivisible operation. +5. Pinned-memory transfer MUST be enabled when the selected processing device benefits from it. CPU-only processing MUST avoid unnecessary pinned copies. +6. Implementations MUST NOT accumulate a complete model, LoRA, or unbounded output batch in RAM. The next work unit may be prefetched while the current unit is processed, but the prefetch bound MUST remain explicit. +7. Every yielded source tensor occurrence MUST be marked processed after its logical work unit completes or aborts. All strong references to source, normalized, padded, device, and output intermediates MUST be removed before advancing to the next work unit. +8. Every async stream and handler MUST close in `finally` or through an equivalent deterministic context manager. Failure paths are subject to the same release rules as successful paths. +9. Saving MUST use UEL incremental writing. Outputs MUST be submitted as they are completed rather than retained in an in-memory state dictionary. +10. File replacement MUST be atomic: write to a temporary sibling file, finalize successfully, then replace the destination. A failure MUST preserve any existing destination and remove the incomplete temporary file. +11. Unsupported low-bit tensors MUST use the shared quantization-preservation path when byte-exact copying is valid. Code MUST NOT silently cast, decode, merge, or relabel unsupported low-bit storage. + +The shared implementations in `nodes/uel_io.py`, `nodes/extraction_stream.py`, and `nodes/quantization_guard.py` are the required starting points. New node-local stream or writer abstractions require a demonstrated capability that the shared implementation cannot provide. + +## LoRA Alpha Normalization for Merge Operations + +1. Every operation that combines a LoRA with another LoRA or applies a LoRA to a model MUST account for per-layer alpha before weighting, padding, concatenation, consensus processing, DARE/TIES processing, or delta application. +2. For a valid factor pair with rank `r`, normalization is performed mathematically as: + + `normalized_up = up * (alpha / r)` + + `normalized_down = down` + + Scaling one factor preserves the represented delta while avoiding unnecessary work and memory. +3. A layer without alpha has an implicit scale of `1.0`. Inputs with and without alpha may be merged only after alpha-bearing pairs have been normalized to this common representation. +4. Normalized merge outputs MUST NOT contain alpha tensors. Alpha MUST NOT be invented for inputs or output formats that do not provide it. +5. Alpha is a scalar control value. Its dtype MUST NOT determine factor computation dtype, merged tensor dtype, or save dtype. +6. Alpha association MUST come from the repository's LoRA pair parser, not from an assumed key spelling. PEFT, Diffusers, ComfyUI, and supported mixed naming forms are governed by the parsed logical pair. +7. Normalization MUST occur before rank padding or other shape alignment. Padding an unnormalized pair and applying its original alpha against the padded rank changes the represented delta. +8. The unscaled factor reference MUST be replaced immediately when normalization creates a new tensor. Both source factors, the alpha scalar, normalized factors, padded factors, and device copies MUST be released when the logical layer completes. +9. A low-bit alpha scalar is valid when both factors use supported floating storage. If an alpha-bearing down or up factor uses unsupported low-bit storage, the operation MUST fail with an actionable error because byte-exact preservation cannot also produce an alpha-free normalized result. +10. Format metadata may describe that normalization occurred, but metadata MUST NOT be used as a substitute for applying the mathematical transformation. + +The shared implementation in `nodes/lora_alpha.py` is mandatory for merge paths. A format-specific implementation may only replace it when the represented delta differs mathematically and that difference is documented and tested. + +## Required Verification + +Any implementation that reads, writes, merges, resizes, extracts, or applies model or LoRA tensors MUST include tests appropriate to its scope. Merge changes involving LoRA factors MUST cover: + +- alpha present and absent in the same operation; +- non-neutral alpha where `alpha / rank` is not `1.0`; +- normalization before padding or concatenation; +- alpha-free output; +- mixed factor ranks and dtypes; +- low-bit scalar alpha acceptance and unsupported low-bit factor rejection; +- stream closure, `mark_processed`, and reference release on success and failure; +- atomic destination preservation on writer failure. + +Repository guards MUST continue to reject production `get_tensor()` calls and direct safetensors imports. The complete test suite, changed-file lint, compilation, and `git diff --check` are required before commit.