convert: mirror the --indir resume params guard onto --repo download loops
Upstream courtesy fix, found while auditing the conversion recipe for this branch -- pre-existing in dev, not introduced by int3-g64, but directly relevant to anyone converting for real (a #383-class gap: same failure family as the resume/manifest work already done for --indir, and the silent-mixing mechanism issue #355 fixed for a narrower case). The --indir path already refuses to resume with different conversion parameters on the same --outdir (a manifest records ebits/xbits/io_bits/ group_size/n_layers/bits_map and compares on every resume). The --repo streaming download loops (main model, --mtp, --indexer) never got the same guard: each shard's resume check is just `if os.path.exists(outp): continue` -- true whether or not THIS run's flags match the flags that produced that shard. A --repo conversion resumed with changed bits (--xbits 3 -> 4 mid-run, say, after an interruption) would silently mix bit-widths across shards in the same container, with no error and no log line distinguishing it from a normal resume. Fix: check_or_record_params(), a small shared helper mirroring the --indir manifest's refuse-on-mismatch logic but without needing its per-shard bookkeeping (the --repo loops already track shard completion correctly via out-NNNNN.safetensors existence, since shard index maps directly to output filename there -- only whether the params used SO FAR still match needed adding). Applied to all three --repo loops with per-mode sidecar files (.out-mtp-params.json / .out-idx-params.json / .out-params.json) so a --mtp and a main-model conversion into the same --outdir don't cross-check each other's parameters. Also includes PROJ_BITS (the per-projection expert bit overrides) in the tracked params dict on BOTH paths -- it was missing from --indir's existing manifest too, so a resume with a changed --up-bits/--gate-bits/ --down-bits would have passed the existing guard silently. Verified directly (no real HF downloads; --repo network paths can't be exercised under this task's constraints): unit-tested check_or_record_params() standalone -- fresh outdir accepts and records, a same-params resume accepts, a changed --xbits is refused, and a proj_bits-only change (nothing else different) is refused. Re-ran the existing --indir dry-run end to end (convert, resume, resume-with-changed- xbits) to confirm the manifest-based path still works correctly with proj_bits added to its params dict. Gates: make test-c (20/20) and make test-python (85/85) both pass.
This commit is contained in:
monotophic
@@ -247,6 +247,32 @@ def convert_shard(path, out_dict, n_layers, ebits, io_bits, xbits,
|
||||
|
||||
def free_gb(p): return shutil.disk_usage(p).free / 1e9
|
||||
|
||||
def check_or_record_params(outdir, prefix, params):
|
||||
"""#383-class guard, mirrored onto the --repo download loops from the --indir
|
||||
path's resume manifest (below): a resumed run with DIFFERENT conversion
|
||||
parameters (bits, group size, PROJ_BITS, ...) must not silently mix bit-widths
|
||||
across shards in the same outdir -- the #355 failure mode (a second pass with
|
||||
changed flags overwriting/interleaving with a finished container in silence).
|
||||
Unlike the --indir manifest this doesn't need to track per-shard completion:
|
||||
the --repo loops already do that via out-NNNNN.safetensors existence, since
|
||||
shard index maps directly to output filename there. Only whether the params
|
||||
used SO FAR match this run's needs checking. Returns False (caller should
|
||||
abort) on a mismatch, True otherwise; records params on first use."""
|
||||
path = os.path.join(outdir, f".{prefix}params.json")
|
||||
if os.path.exists(path):
|
||||
try: prev = json.loads(open(path).read())
|
||||
except (OSError, ValueError): prev = None
|
||||
if prev is not None and prev != params:
|
||||
print(f"ERROR: {path} records a conversion with {prev};\n"
|
||||
f" this run uses {params}. Refusing to mix conversions in the "
|
||||
f"same outdir — use a fresh --outdir (or delete {path} and the "
|
||||
f"{prefix}*.safetensors shards to redo).")
|
||||
return False
|
||||
tmp = path + ".tmp"
|
||||
with open(tmp, "w") as f: json.dump(params, f, indent=1) # atomic write, same reasoning as the --indir manifest
|
||||
os.replace(tmp, path)
|
||||
return True
|
||||
|
||||
def main():
|
||||
ap = argparse.ArgumentParser()
|
||||
ap.add_argument("--repo", default=None)
|
||||
@@ -440,7 +466,8 @@ def main():
|
||||
# EN: resume skips only what matches, and different parameters on the same
|
||||
# EN: outdir are refused instead of mixing containers (the #355 failure mode).
|
||||
params = {"ebits": a.ebits, "io_bits": a.io_bits, "xbits": a.xbits,
|
||||
"group_size": a.group_size, "n_layers": a.n_layers, "bits_map": bits_map}
|
||||
"group_size": a.group_size, "n_layers": a.n_layers, "bits_map": bits_map,
|
||||
"proj_bits": dict(PROJ_BITS)}
|
||||
prog_path = os.path.join(a.outdir, f".{prefix}progress.json")
|
||||
prog = {}
|
||||
if os.path.exists(prog_path):
|
||||
@@ -718,6 +745,10 @@ def main():
|
||||
except Exception: pass
|
||||
tmp = os.path.join(a.outdir, "_inflight"); os.makedirs(tmp, exist_ok=True)
|
||||
if a.mtp:
|
||||
params = {"ebits": a.ebits, "io_bits": a.io_bits, "xbits": a.xbits,
|
||||
"group_size": a.group_size, "n_layers": a.n_layers, "bits_map": bits_map,
|
||||
"proj_bits": dict(PROJ_BITS)}
|
||||
if not check_or_record_params(a.outdir, "out-mtp-", params): return
|
||||
import urllib.request
|
||||
idx = json.loads(urllib.request.urlopen(
|
||||
f"https://huggingface.co/{a.repo}/resolve/main/model.safetensors.index.json", timeout=30).read())["weight_map"]
|
||||
@@ -737,6 +768,10 @@ def main():
|
||||
print(f" -> {os.path.basename(outp)} ({os.path.getsize(outp)/1e9:.2f} GB, {len(out)} tensors)", flush=True)
|
||||
shutil.rmtree(tmp, ignore_errors=True); print("[MTP] DONE."); return
|
||||
if a.indexer:
|
||||
params = {"ebits": a.ebits, "io_bits": a.io_bits, "xbits": a.xbits,
|
||||
"group_size": a.group_size, "n_layers": a.n_layers, "bits_map": bits_map,
|
||||
"proj_bits": dict(PROJ_BITS)}
|
||||
if not check_or_record_params(a.outdir, "out-idx-", params): return
|
||||
import urllib.request
|
||||
idx = json.loads(urllib.request.urlopen(
|
||||
f"https://huggingface.co/{a.repo}/resolve/main/model.safetensors.index.json", timeout=30).read())["weight_map"]
|
||||
@@ -756,6 +791,10 @@ def main():
|
||||
if os.path.isfile(blob): os.remove(blob)
|
||||
print(f" -> {os.path.basename(outp)} ({len(out)} tensors)", flush=True)
|
||||
shutil.rmtree(tmp, ignore_errors=True); print("[IDX] DONE."); return
|
||||
params = {"ebits": a.ebits, "io_bits": a.io_bits, "xbits": a.xbits,
|
||||
"group_size": a.group_size, "n_layers": a.n_layers, "bits_map": bits_map,
|
||||
"proj_bits": dict(PROJ_BITS)}
|
||||
if not check_or_record_params(a.outdir, "out-", params): return
|
||||
for i, sh in enumerate(shards):
|
||||
if free_gb(a.outdir) < a.min_free_gb:
|
||||
print(f"STOP: free space is below {a.min_free_gb} GB. Free space and rerun to resume."); break
|
||||
|
||||
Reference in New Issue
Block a user