-
Notifications
You must be signed in to change notification settings - Fork 3
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.
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
Add spec v2 #4
Add spec v2 #4
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good. Just some minor comments. A good start!
// | ||
// This structure is designed to be flexible and accommodate various transformer architectures | ||
// used in state-of-the-art language models. | ||
type TransformerForCausalLLM struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not referenced anywhere. Let's add it when config.go references it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
MediaTypeModelConfig = "application/vnd.cnai.model.config.v2+json" | ||
|
||
// MediaTypeModelLicense specifies the media type for model license. | ||
MediaTypeModelLicense = "application/vnd.cnai.model.license.v2+json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the license file in json format? Looks like plaintext
is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
const ( | ||
// MediaTypeModelProcessorText specifies the media type for text processors. | ||
// This includes tokenizers like sentencepiece, used for processing textual input. | ||
MediaTypeModelProcessorText = "application/vnd.cnai.model.processor.text.v2.tar" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The processors are source code files right? Let's compress it with tar+gzip
or tar+zstd
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I see that zstd is still wip.
No description provided.