- 
                Notifications
    
You must be signed in to change notification settings  - Fork 248
 
[1/n llava]unify model construction ppl #1153
Changes from all commits
fff8647
              4b666a7
              cc8b4d6
              7ec018a
              94e56f1
              2e3d1dc
              63d76a1
              01bb624
              319ac86
              141fea0
              8cd0936
              304fece
              1eff939
              a356897
              f224da7
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -27,12 +27,6 @@ | |
| 
     | 
||
| from PIL import Image | ||
| 
     | 
||
| # torchtune model definition dependencies | ||
| from torchtune.data import Message | ||
| from torchtune.generation._generation import sample as tune_sample | ||
| from torchtune.models.llama3 import llama3_tokenizer | ||
| from torchtune.training import set_default_dtype | ||
| 
     | 
||
| from torchchat.cli.builder import ( | ||
| _initialize_model, | ||
| _initialize_tokenizer, | ||
| 
        
          
        
         | 
    @@ -43,6 +37,12 @@ | |
| from torchchat.utils.build_utils import device_sync, set_precision | ||
| from torchchat.utils.device_info import get_device_info | ||
| 
     | 
||
| # torchtune model definition dependencies | ||
| from torchtune.data import Message | ||
| from torchtune.generation._generation import sample as tune_sample | ||
| from torchtune.models.llama3 import llama3_tokenizer | ||
| from torchtune.training import set_default_dtype | ||
| 
     | 
||
| 
     | 
||
| class _ChatFormatter(ABC): | ||
| def __init__(self, tokenizer): | ||
| 
          
            
          
           | 
    @@ -795,16 +795,12 @@ def chat( | |
| 
     | 
||
| # This is a hack to get around the fact that different models have different ways to record their max_seq_length and might be wrong | ||
| # TODO: unify the max_seq_length config representation. | ||
| if generator_args.is_torchtune_model: | ||
| max_seq_length = self.model.config.transformer_args.get("text", {}).get( | ||
| "max_seq_len", 2048 | ||
| ) | ||
| elif generator_args.chat_mode: | ||
| if ( | ||
| max_seq_length := self.model.config.transformer_args.get("text", None) | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Your changes are right; just calling out that the old implementation was broken in 26c1d8b  | 
||
| is None | ||
| ): | ||
| max_seq_length = 2048 | ||
| text_transformer_args = self.model.text_transformer_args | ||
| max_seq_length = ( | ||
| text_transformer_args.max_seq_length if text_transformer_args else 2048 | ||
| ) | ||
| 
     | 
||
| if generator_args.chat_mode: | ||
| print( | ||
| f"Entering Chat Mode. Will continue chatting back and forth with the language model until the models max context length of {max_seq_length} tokens is hit or until the user says /bye" | ||
| ) | ||
| 
        
          
        
         | 
    @@ -814,8 +810,7 @@ def chat( | |
| if get_system_prompt == "y" or get_system_prompt == "Y": | ||
| self.system_prompt = input("What is your system prompt? \n") | ||
| 
     | 
||
| else: | ||
| text_transformer_args = self.model.config.transformer_args.get("text", None) | ||
| elif not generator_args.is_torchtune_model: | ||
| max_seq_length = min( | ||
| encoded.size(0) + generator_args.max_new_tokens, | ||
| ( | ||
| 
          
            
          
           | 
    ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -163,50 +163,64 @@ def from_params(cls, params): | |
| 
     | 
||
| @dataclass | ||
| class ModelArgs: | ||
| """ | ||
| A data class to describe the structure of a model. | ||
| Attributes: | ||
| model_type (ModelType): The type of the model. This attribute is used to categorize the model into different classes. | ||
| transformer_args (Dict[str, Dict[str, Any]]): A dictionary containing the parameters for each transformer in the model. | ||
| The outer dictionary has transformer names as keys and inner dictionaries as values. Each inner dictionary contains | ||
| the parameter names and their corresponding values for the respective transformer. | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
 This sounds like the intent of transformer args; why can't we use that instead of Dictp[str, Any] There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for unification. this arg takes charge for describing architecture for all models, including tune-backends, chat-backends, and even mix-backends. so we need a unify way to descible how we will set up them.  | 
||
| TODO: econcile Dict[str, Any] into tranformer-arg-family classes in future PRs. | ||
| 
     | 
||
| use_tiktoken (bool): A flag indicating whether to use TikToken as the tokenizer for the model. | ||
| Note: | ||
| It is recommended to use factory functions to create instances of this class instead of directly using the constructor. | ||
| """ | ||
| 
     | 
||
| model_type: ModelType | ||
| transformer_args: Dict[str, Union[Dict, TransformerArgs]] | ||
| transformer_args: Dict[str, Dict[str, Any]] | ||
                
      
                  Jack-Khuu marked this conversation as resolved.
               
          
            Show resolved
            Hide resolved
         | 
||
| use_tiktoken: bool | ||
| 
     | 
||
| def __init__( | ||
| self, | ||
| transformer_args: Union[TransformerArgs, Dict[str, TransformerArgs]], | ||
| transformer_args: Dict[str, Dict[str, Any]], | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should find a way to reconcile  This makes this work well since we have 3 "cases", but storing/passing around an untyped Dict makes me nervous There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More than agree. My mental model would be creating an abstract class containig essential apis for all module configurations, and for different transformer (e.g. ours, tunes, etc) we have a different implementation. Dict[str, Any] is not a great way.  | 
||
| model_type: ModelType = ModelType.TextOnly, | ||
| use_tiktoken: bool = False, | ||
| ) -> None: | ||
| self._sanity_check(transformer_args, model_type) | ||
| 
     | 
||
| self.model_type = model_type | ||
| if isinstance(transformer_args, TransformerArgs): | ||
| assert model_type == ModelType.TextOnly | ||
| self.transformer_args = {"text": transformer_args} | ||
| else: | ||
| self.transformer_args = transformer_args | ||
| self.transformer_args = transformer_args | ||
| 
     | 
||
| # Model-level attributes | ||
| self.use_tiktoken = use_tiktoken | ||
| 
     | 
||
| def _sanity_check( | ||
| self, | ||
| transformer_args: Union[TransformerArgs, Dict[str, TransformerArgs]], | ||
| transformer_args: Dict[str, Dict[str, Any]], | ||
| model_type: ModelType, | ||
| ) -> None: | ||
| assert isinstance(model_type, ModelType) | ||
| assert isinstance(transformer_args, (TransformerArgs, dict)) | ||
| assert isinstance(model_type, ModelType), model_type | ||
| assert isinstance(transformer_args, dict) | ||
| 
     | 
||
| @classmethod | ||
| def from_params(cls, params_path): | ||
| with open(params_path, "r") as f: | ||
| loaded_params = json.loads(f.read()) | ||
| 
     | 
||
| try: | ||
| # try to interpret as a single transformer config | ||
| transformer_args: Dict[str, TransformerArgs] = {} | ||
| transformer_args["text"] = TransformerArgs.from_params(loaded_params) | ||
| if (model_type := loaded_params.get("model_type", None)) is None: | ||
| model_type = ModelType.TextOnly | ||
| 
     | 
||
| except TypeError: | ||
| # try to interpret as a dict of transformer configs | ||
| model_type = ModelType(loaded_params["model_type"]) | ||
| 
     | 
||
| if (model_type_name := loaded_params.get("model_type", None)) is None: | ||
| # The model params is in the transformer_args format | ||
| # set the model_type to TextOnly and reformat the params | ||
| model_type = ModelType.TextOnly | ||
| transformer_args = {"text": loaded_params} | ||
| else: | ||
| model_type = ModelType(model_type_name) | ||
| transformer_args = { | ||
| k: v for k, v in loaded_params.items() if k != "model_type" | ||
| } | ||
| return cls(transformer_args, model_type) | ||
| 
     | 
||
| use_tiktoken = loaded_params.get("use_tiktoken", False) | ||
| return cls(transformer_args, model_type, use_tiktoken) | ||
| 
     | 
||
| @classmethod | ||
| def from_table(cls, name: str): | ||
| 
          
            
          
           | 
    @@ -293,6 +307,10 @@ def __init__(self, config: ModelArgs) -> None: | |
| self.config = config | ||
| self.model = self.build_model() | ||
| 
     | 
||
| # text_transformer_args represents the args for the text transformer in the model. | ||
| # It should be assigned in the actual model implementation, if any. | ||
| self.text_transformer_args = None | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment on this since it is a special case  | 
||
| 
     | 
||
| def build_model(self) -> nn.Module: | ||
| """ | ||
| Builds a model based on the provided configuration. | ||
| 
        
          
        
         | 
    @@ -304,10 +322,11 @@ def build_model(self) -> nn.Module: | |
| recipe = ModelRecipe.get_recipe(self.config.model_type) | ||
| modules = {} | ||
| for name, module_class in recipe.modules.items(): | ||
| if isinstance(config_args := self.config.transformer_args[name], dict): | ||
| modules[name] = module_class(**config_args) | ||
| config_args = self.config.transformer_args[name] | ||
| if module_class == Transformer: | ||
| modules[name] = module_class(TransformerArgs.from_params(config_args)) | ||
| else: | ||
| modules[name] = module_class(config_args) | ||
| modules[name] = module_class(**config_args) | ||
| 
     | 
||
| return recipe.fusion_class(**modules) | ||
| 
     | 
||
| 
          
            
          
           | 
    @@ -353,6 +372,10 @@ def from_gguf(cls, gguf_path: str, **kwargs): | |
| 
     | 
||
| 
     | 
||
| class TextOnlyModel(Model): | ||
| def __init__(self, config: ModelArgs) -> None: | ||
| super().__init__(config) | ||
| self.text_transformer_args = self.model.config | ||
| 
     | 
||
| def forward(self, tokens: Tensor, input_pos: Optional[Tensor] = None) -> Tensor: | ||
| return self.model(tokens, input_pos) | ||
| 
     | 
||
| 
          
            
          
           | 
    @@ -391,6 +414,7 @@ def reset_caches(self): | |
| self.model.reset_caches() | ||
| 
     | 
||
| 
     | 
||
| 
     | 
||
| MODEL_TYPE_TO_CLASS = { | ||
| ModelType.TextOnly: TextOnlyModel, | ||
| ModelType.Flamingo: FlamingoModel, | ||
| 
          
            
          
           | 
    @@ -781,6 +805,8 @@ def __init__(self, config, path) -> None: | |
| self.config = config | ||
| self.model_ = exec_lib._load_for_executorch(str(path)) | ||
| 
     | 
||
| self.text_transformer_args = TransformerArgs.from_params(self.config.transformer_args["text"]) | ||
| 
     | 
||
| def forward(self, x, input_pos): | ||
| # model_.forward expects inputs to be wrapped in a tuple | ||
| forward_inputs = (x.to(torch.long), input_pos.to(torch.long)) | ||
| 
        
          
        
         | 
    @@ -794,6 +820,6 @@ def forward(self, x, input_pos): | |
| 
     | 
||
| def setup_caches(self, max_batch_size, max_seq_length): | ||
| pass | ||
| 
     | 
||
| except: | ||
| pass | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| { | ||
| "model_type": "llama3_1", | ||
| "use_tiktoken": true, | ||
| "text": { | ||
| "vocab_size": 128256, | ||
| "num_layers": 80, | ||
| 
          
            
          
           | 
    ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| { | ||
| "model_type": "llama3_1", | ||
| "use_tiktoken": true, | ||
| "text": { | ||
| "vocab_size": 128256, | ||
| "num_layers": 32, | ||
| 
          
            
          
           | 
    ||
Uh oh!
There was an error while loading. Please reload this page.