Skip to content
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

Jump to bottom

Add BERT family of models #15

Merged
merged 10 commits into from
Feb 13, 2024
Merged

Add BERT family of models #15

merged 10 commits into from
Feb 13, 2024

Conversation

ashdtu
Copy link
Contributor

@ashdtu ashdtu commented Feb 3, 2024 •
edited
Loading

Related Issues

Issue: #12

Changes

  • Implement BERT family of models for obtaining text embeddings from input.
  • Support for downloading model checkpoints automatically
  • Support for loading multiple Bert variants with roberta-base as the default model.

antimora
antimora reviewed Feb 4, 2024
View reviewed changes
Copy link
Collaborator

@antimora antimora left a comment

Choose a reason for hiding this comment

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

Briefly looked at the code. It's great that we are adding more models.

See my inlined comments.

bert-burn/examples/infer-embedding.rs Outdated Show resolved Hide resolved
bert-burn/src/bert.rs Outdated Show resolved Hide resolved
bert-burn/src/data/batcher.rs Outdated Show resolved Hide resolved
nathanielsimard
nathanielsimard requested changes Feb 5, 2024
View reviewed changes
Copy link
Member

@nathanielsimard nathanielsimard left a comment

Choose a reason for hiding this comment

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

Very nice addition to have BERT models in here. See my comments bellow for improvements.

bert-burn/LICENSE-APACHE Outdated Show resolved Hide resolved
bert-burn/LICENSE-MIT Outdated Show resolved Hide resolved
bert-burn/examples/infer-embedding.rs Outdated Show resolved Hide resolved
bert-burn/examples/infer-embedding.rs Outdated Show resolved Hide resolved
bert-burn/src/bert.rs Outdated Show resolved Hide resolved
bert-burn/src/bert.rs Outdated Show resolved Hide resolved
bert-burn/src/data/batcher.rs Outdated Show resolved Hide resolved
bert-burn/src/data/batcher.rs Outdated Show resolved Hide resolved
bert-burn/src/data/batcher.rs Outdated Show resolved Hide resolved
bert-burn/src/data/batcher.rs Outdated Show resolved Hide resolved
@ashdtu
Copy link
Contributor Author

ashdtu commented Feb 9, 2024

Thanks @antimora and @nathanielsimard for all the comments. Made the required changes and some enhancements. Let me know if this looks good to go.

nathanielsimard
nathanielsimard requested changes Feb 10, 2024
View reviewed changes
Copy link
Member

@nathanielsimard nathanielsimard left a comment

Choose a reason for hiding this comment

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

Very small changes regarding feature flags and a small API adjustment. When fixed, I think it's going to be ready to merge! 🎉

bert-burn/Cargo.toml Outdated Show resolved Hide resolved
bert-burn/Cargo.toml Outdated Show resolved Hide resolved
bert-burn/README.md Outdated Show resolved Hide resolved
bert-burn/src/loader.rs Outdated Show resolved Hide resolved
bert-burn/src/loader.rs Outdated Show resolved Hide resolved
@ashdtu ashdtu merged commit b225ed6 into main Feb 13, 2024
2 checks passed
@ashdtu ashdtu deleted the bert branch February 13, 2024 15:43
@ashdtu ashdtu mentioned this pull request Feb 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antimora antimora antimora left review comments

@nathanielsimard nathanielsimard nathanielsimard approved these changes

@louisfd louisfd Awaiting requested review from louisfd

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ashdtu @antimora @nathanielsimard