Skip to content

refactor: share tensor writer across ggml converters - #19

Merged
PABannier merged 1 commit into
mainfrom
cleanup/share-converter-writer
Sep 16, 2026
Merged

PABannier merged 1 commit into
mainfrom
cleanup/share-converter-writer

Conversation

@PABannier

Copy link
Copy Markdown
Owner

What / why

convert_sam3_to_ggml.py, convert_sam2_to_ggml.py and convert_edgetam_to_ggml.py each had their own copy of write_tensor (the 32-byte-aligned tensor record writer) and of the FTYPE_F32/F16 constants. The copies differed only in which name substrings are kept as f32.

This PR moves that code into a new ggml_writer.py at the repo root. write_tensor(fout, name, data, ftype, keep_f32=KEEP_F32) takes the substrings as a parameter:

  • sam2 uses the default KEEP_F32
  • sam3 passes KEEP_F32 + ("freqs_cis",)
  • edgetam passes KEEP_F32 + ("latents",)

"pos_embed" is no longer listed separately because "embed" already matches it. rename_key and write_header really do differ between the converters, so they stay where they are. The now-unused numpy import is removed from the sam2 and edgetam converters.

Verification

A throwaway script (not committed) pulled the old write_tensor and FTYPE constants out of each file at origin/main with ast. It compared them against the new shared function, called with the extra arguments read from each converter's real write_tensor(...) call. Every write went to a BytesIO and the bytes were compared. Test matrix:

  • names: every keep-f32 substring, pos_embed, freqs_cis, latents, perceiver.latents_2d, normal .weight / .bias names
  • shapes: 0-D, 1-D, 2-D, 3-D, 4-D, all-ones 4-D, and a non-contiguous transposed array
  • dtypes: float32, float64, float16, int64
  • ftype 0 and 1
  • starting offsets 0, 1, 5, 12, 31, 32, 33, 63 (to exercise alignment)
  • plus one multi-record stream per converter

Result: 20,160 cases (6,720 per converter) plus 3 streams, all byte-identical.

I also checked that every file parses with ast and passes py_compile. --help runs for all three converters when invoked by absolute path from a different cwd, with only numpy installed (torch is imported lazily inside main). That matches how scripts/download_model.sh calls them; the README uses uv run python convert_*.py from the repo root.

Diff

4 files changed, 26 insertions(+), 137 deletions(-)

Caveats

  • The converters now depend on ggml_writer.py being in the same directory. That holds for every documented invocation (README, scripts/download_model.sh), but copying a single converter somewhere else on its own will no longer work.
  • No real checkpoint was converted end to end; equivalence rests on the byte-compare above.

🤖 Generated with Claude Code

https://claude.ai/code/session_01M2qLmQ9ag49hT61V6i9qAq

convert_sam3/sam2/edgetam_to_ggml.py each carried a copy of write_tensor
and the FTYPE constants. Move them to ggml_writer.py, parameterised by the
keep-f32 name substrings (sam3 adds "freqs_cis", edgetam adds "latents").
Output bytes are unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M2qLmQ9ag49hT61V6i9qAq
@PABannier
PABannier merged commit 1c4c6e4 into main Sep 16, 2026
3 checks passed
@PABannier
PABannier deleted the cleanup/share-converter-writer branch September 16, 2026 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant