Added Phi #132
Added Phi #132
Conversation
| @@ -325,6 +328,19 @@ def get_model( | |||
| trust_remote_code=trust_remote_code, | |||
| ) | |||
| raise NotImplementedError("Qwen model requires flash attention v2") | |||
|
|
|||
| if model_type == "phi-msft" or model_type == "phi": | |||
There was a problem hiding this comment.
what do you think about using model_type in [] instead ?
| super().__init__() | ||
| self.num_heads = config.n_head | ||
| self.hidden_size = config.n_embd | ||
| self.head_size = self.hidden_size // self.num_heads |
There was a problem hiding this comment.
Is it intentional to use self.num_heads like this here but then update the value of self.num_heads by the number of process groups in line 110?
There was a problem hiding this comment.
Yes, when creating the modules we need the num_heads value pre-splitting. Then later on we use num_heads post-split.
| revision=revision, | ||
| padding_side="left", | ||
| truncation_side="left", | ||
| trust_remote_code=trust_remote_code, |
There was a problem hiding this comment.
Since we're adding trust_remote_code, do we already install the einops library to convert weights when using microsoft/phi-1.5?
There was a problem hiding this comment.
trust_remote_code is False by default. einops is installed already (for Falcon). This current implementation works with the microsoft version of the weights, rather than the changes HF made.
Example:
Prompt:
curl 127.0.0.1:8080/generate \ -X POST \ -d '{"inputs": "Instruct: Write a detailed analogy between mathematics and a lighthouse.\nOutput:", "parameters": {"max_new_tokens": 64}}' \ -H 'Content-Type: application/json'Response: