Skip to content

strata_pack.py build --force: unmake the old pack before writing, so a build that stops part-way is an unfinished build, not a pack (#634 follow-up) #1176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Avicennasis wants to merge 1 commit into from
Closed

strata_pack.py build --force: unmake the old pack before writing, so a build that stops part-way is an unfinished build, not a pack (#634 follow-up) #1176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion tools/strata_pack.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,13 @@ def build(gguf: pathlib.Path, out_dir: pathlib.Path, n_layers: int | None, skip_
"--out, or --force to rebuild it" % (out_dir, marker.name))
return 1
out_dir.mkdir(parents=True, exist_ok=True)
if force:
# the old pack's markers go before a byte is written, so a forced build that stops part-way leaves an
# unfinished build (experts.bin without a manifest.json), which the next build or setup makes again - not
# the old manifest.json and index.txt over partly new experts, which setup's "index.txt and experts.bin are
# there" check would hand to the engine. The data files need no removing: the build rewrites each one.
for name in PACK_MARKERS:
(out_dir / name).unlink(missing_ok=True)
g = G.GGUFFile(gguf)
head, flen = open_shard(gguf)
data_off = data_section_offset(g, flen)
Expand Down Expand Up @@ -423,7 +430,7 @@ def main() -> int:
b.add_argument("--out", required=True)
b.add_argument("--layers", type=int, default=None, help="expert layers to emit (default 48)")
b.add_argument("--skip-hash", action="store_true")
b.add_argument("--force", action="store_true", help="build into a directory that already holds a pack (#634)")
b.add_argument("--force", action="store_true", help="build into a directory that already holds a pack, unmaking that pack first (#634)")
v = sub.add_parser("verify")
v.add_argument("--gguf", required=True)
v.add_argument("--out", required=True)
Expand Down
15 changes: 15 additions & 0 deletions tools/test_shards.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,21 @@ def test_force_builds_into_an_occupied_dir(self):
with contextlib.redirect_stdout(buf), self.assertRaises(Stop):
strata_pack.build(self.dir / "m-00001-of-00002.gguf", out, None, True, True)

def test_force_unmakes_the_old_pack_before_building(self):
# #634 follow-up: --force drops the old pack's markers before the build opens the GGUF, so a forced build
# that stops part-way is an unfinished build (made again next time), not the old index.txt and manifest.json
# over partly new experts; the data files are left to the build, which rewrites each one
out = self.dir / "forced-markers"
out.mkdir()
for name in strata_pack.PACK_MARKERS:
(out / name).write_text("the pack that was there", encoding="utf-8")
(out / "experts.bin").write_bytes(b"its experts")
buf = io.StringIO()
with contextlib.redirect_stdout(buf), self.assertRaises(Stop):
strata_pack.build(self.dir / "m-00001-of-00002.gguf", out, None, True, True)
self.assertEqual([p.name for p in out.iterdir()], ["experts.bin"])
self.assertEqual((out / "experts.bin").read_bytes(), b"its experts")

def test_unfinished_build_is_rebuilt(self):
# experts.bin without manifest.json is a build that stopped before its last file: not a pack, rebuilt as before
out = self.dir / "unfinished"
Expand Down