Skip to content

feat(bitnet/model): implement model weights and tokenizer integration - #197

Merged
thejhh merged 41 commits into
bitnetfrom
172-acquire-model-weights-tokenizer
May 20, 2025
Merged

feat(bitnet/model): implement model weights and tokenizer integration#197
thejhh merged 41 commits into
bitnetfrom
172-acquire-model-weights-tokenizer

Conversation

@thejhh

@thejhh thejhh commented May 19, 2025

Copy link
Copy Markdown
Contributor

Changes

  • Added tokenizer support with BPE (Byte Pair Encoding) implementation
  • Updated model loader to handle GGUF format
  • Added comprehensive tests and benchmarks for both tokenizer and model loader
  • Created download script for model weights and tokenizer
  • Fixed path handling in model loader and tokenizer

Test Coverage

  • Current coverage: 77.9% for model package
  • Coverage changes: → 77.9%
  • Untested areas:
    • Internal config package (0% coverage)
    • Math operations package (0% coverage)

Performance Metrics

Memory Usage

  • Allocations per operation:
    • Tokenizer operations: 0-2 allocs/op
    • Model loading: 1-3 allocs/op
    • Chunk loading: 1 alloc/op

CPU Performance

  • Operation timing:
    • Tokenization: ~100ns/op
    • Model loading: ~1ms/op
    • Chunk loading: ~10μs/op

Areas for Improvement

High Priority

  • Add tests for internal packages
  • Optimize tokenizer memory usage
  • Implement memory pooling for large tensors

Medium Priority

  • Improve error handling in model operations
  • Add more comprehensive benchmarks
  • Enhance documentation

Low Priority

  • Consider SIMD optimizations
  • Add more tensor operations
  • Improve test organization

@thejhh thejhh added the enhancement New feature or request label May 20, 2025
@thejhh thejhh changed the title feat: Acquire Model Weights & Tokenizer feat(bitnet/model): implement model weights and tokenizer integration May 20, 2025
@thejhh thejhh linked an issue May 20, 2025 that may be closed by this pull request

@thejhh thejhh left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix review comments

Comment thread pkg/bitnet/internal/model/loader.go Outdated
Comment thread pkg/bitnet/model/model.go Outdated
Comment thread pkg/bitnet/model/tokenizer.go Outdated
Comment thread pkg/bitnet/internal/model/loader.go Outdated
Comment thread pkg/bitnet/internal/model/tokenizer.go Outdated
Comment thread pkg/bitnet/internal/model/tokenizer.go Outdated
Comment thread pkg/bitnet/internal/model/loader.go Outdated
Comment thread pkg/bitnet/model/embedded/embedded.go Outdated
Comment thread pkg/bitnet/model/model.go Outdated
Comment thread pkg/bitnet/internal/model/loader.go Outdated
Jaakko Heusala added 7 commits May 20, 2025 19:21
…management -- use absolute paths, fix chunk pool, improve loader tests and error handling
… -- loads from correct path, covers unknown words, decoding, special tokens, removes BPE fallback for unknown words
…net/model -- all logic now lives in internal/model

@thejhh thejhh left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to make changes I've described in the full review comments.

Comment thread pkg/bitnet/internal/model/loader.go Outdated
Comment thread pkg/bitnet/model/tokenizer.go Outdated
Comment thread pkg/bitnet/internal/model/loader.go Outdated
Comment thread pkg/bitnet/internal/model/loader.go Outdated
Comment thread pkg/bitnet/internal/model/loader.go Outdated
Comment thread pkg/bitnet/internal/model/tokenizer.go Outdated
Comment thread pkg/bitnet/internal/model/tokenizer.go Outdated
…fs dependency, unified model/tokenizer loading, test/bench isolation)

@thejhh thejhh left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error handling requires some work still. See full review comments.

Comment thread pkg/bitnet/internal/model/loader.go Outdated
Comment thread pkg/bitnet/internal/model/tokenizer.go Outdated
@thejhh

thejhh commented May 20, 2025

Copy link
Copy Markdown
Contributor Author

We need to test all code:

?   	github.com/hyperifyio/gnd/pkg/bitnet/internal/config	[no test files]
?   	github.com/hyperifyio/gnd/pkg/bitnet/internal/math	[no test files]

@thejhh

thejhh commented May 20, 2025

Copy link
Copy Markdown
Contributor Author

Some tests are also failing:

=== RUN   TestNewTokenizerErrors/file_not_found
--- FAIL: TestNewTokenizerErrors (0.00s)
    --- PASS: TestNewTokenizerErrors/nil_filesystem (0.00s)
    --- FAIL: TestNewTokenizerErrors/empty_model_path (0.00s)
    --- PASS: TestNewTokenizerErrors/file_not_found (0.00s)

@thejhh

thejhh commented May 20, 2025

Copy link
Copy Markdown
Contributor Author

This is failing too:

=== RUN   TestNewModelLoaderErrors/file_not_found
--- FAIL: TestNewModelLoaderErrors (0.00s)
    --- PASS: TestNewModelLoaderErrors/nil_filesystem (0.00s)
    --- FAIL: TestNewModelLoaderErrors/empty_model_path (0.00s)
    --- PASS: TestNewModelLoaderErrors/file_not_found (0.00s)

@thejhh thejhh left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still some review comments unresolved.

Also fix missing tests:

?   	github.com/hyperifyio/gnd/pkg/bitnet/internal/config	[no test files]
?   	github.com/hyperifyio/gnd/pkg/bitnet/internal/math	[no test files]

Comment thread pkg/bitnet/internal/model/tokenizer.go Outdated
Comment thread pkg/bitnet/internal/model/tokenizer.go Outdated
@thejhh
thejhh requested a review from Copilot May 20, 2025 17:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR restructures and enhances the BitNet project’s .cursor/rules documentation by removing outdated guidelines, adding new standardized procedures, and expanding workflow and benchmark guidance.

  • Removed deprecated rule files.
  • Introduced new PR update, review, and creation templates.
  • Updated project overview, environment setup, branching strategy, and benchmarking guidelines.

Reviewed Changes

Copilot reviewed 38 out of 38 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.cursor/rules/bitnet-pr-update-procedures.mdc New standardized PR update procedures
.cursor/rules/bitnet-pr-review-workflow.mdc Added PR review workflow guidelines
.cursor/rules/bitnet-pr-creation-description.mdc Added PR creation & description format template
.cursor/rules/bitnet-overview.mdc Updated high-level project overview and goals
.cursor/rules/bitnet-environment.mdc Updated development environment setup instructions
.cursor/rules/bitnet-branching-strategy.mdc Defined branch creation and naming conventions
.cursor/rules/bitnet-benchmarks.mdc Defined benchmark file placement and naming rules
.cursor/rules/bitnet-benchmark-invocation.mdc Added benchmark invocation and profiling commands
.cursor/rules/bitnet-benchmark-categories.mdc Introduced benchmark semantic categories
.cursor/rules/bitnet-benchmark-analysis.mdc Added guidance for interpreting and tracking results
Comments suppressed due to low confidence (8)

Comment thread .cursor/rules/bitnet-benchmarks.mdc Outdated

@thejhh thejhh left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still have broken tests:

--- FAIL: TestTokenize (0.00s)
    --- PASS: TestTokenize/known_words (0.00s)
    --- FAIL: TestTokenize/unknown_word (0.00s)
    --- PASS: TestTokenize/empty_text (0.00s)

We also are missing tests for pkg/bitnet/internal/math/ops.go and pkg/bitnet/internal/config/config.go.

@thejhh thejhh left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok , finally we can move on :)

@thejhh
thejhh marked this pull request as ready for review May 20, 2025 19:12
@thejhh
thejhh merged commit 7cd6cf2 into bitnet May 20, 2025
@thejhh thejhh mentioned this pull request May 21, 2025
10 tasks
@thejhh thejhh added the bitnet BitNet implementation label May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bitnet BitNet implementation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Acquire Model Weights & Tokenizer

2 participants