feat: support parsing dtype field from config.#1118
feat: support parsing dtype field from config.#1118DongheJin wants to merge 1 commit intojd-opensource:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a normalize_config_torch_dtype function to standardize the handling of dtype and torch_dtype in model configuration files. Specifically, it ensures that if a configuration only provides dtype, it is also used for torch_dtype when loading model and quantization arguments. A new test case has been added to validate this behavior. The reviewer pointed out that the current implementation of normalize_config_torch_dtype involves an inefficient serialization/deserialization roundtrip for JsonReader creation, suggesting a more direct construction method for performance improvement.
| } | ||
|
|
||
| JsonReader normalized_reader; | ||
| normalized_reader.parse_text(config.dump()); |
There was a problem hiding this comment.
The current implementation of normalize_config_torch_dtype creates a new JsonReader by first serializing the nlohmann::json object to a string (config.dump()) and then deserializing it (normalized_reader.parse_text()). This roundtrip serialization and deserialization is inefficient and introduces unnecessary overhead. It would be more performant if the JsonReader class provided a constructor that directly accepts an nlohmann::json object, allowing for direct construction without the intermediate string conversion.
No description provided.