Conversation
Summary of ChangesHello @zianglih, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new utility script to facilitate the conversion of Hugging Face safetensors checkpoints to the NVFP4 format. The primary objective is to enhance the efficiency of large language models, particularly those utilizing Mixture of Experts architectures, by reducing their memory footprint and potentially accelerating inference. The script intelligently applies quantization to specific model components (MoE expert GEMMs) and ensures that the model's configuration files are appropriately updated to reflect these changes, streamlining the deployment of optimized models. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a script for converting Hugging Face models to the NVFP4 format, with a specific focus on Mixture-of-Experts (MoE) layers. The script is well-organized, handling file operations, model configuration updates, and memory management effectively. My review highlights a potential correctness issue in the quantization logic related to inconsistent boundary handling and suggests improving the robustness of file I/O operations by using with statements.
| result = torch.zeros_like(x, dtype=torch.uint8) | ||
| result[(x >= 0.0) & (x <= 0.25)] = 0 | ||
| result[(x > 0.25) & (x < 0.75)] = 1 | ||
| result[(x >= 0.75) & (x <= 1.25)] = 2 | ||
| result[(x > 1.25) & (x < 1.75)] = 3 | ||
| result[(x >= 1.75) & (x <= 2.5)] = 4 | ||
| result[(x > 2.5) & (x < 3.5)] = 5 | ||
| result[(x >= 3.5) & (x <= 5.0)] = 6 | ||
| result[x > 5.0] = 7 | ||
|
|
||
| result[(x >= -0.25) & (x < -0.0)] = 8 | ||
| result[(x < -0.25) & (x > -0.75)] = 9 | ||
| result[(x <= -0.75) & (x >= -1.25)] = 10 | ||
| result[(x < -1.25) & (x > -1.75)] = 11 | ||
| result[(x <= -1.75) & (x >= -2.5)] = 12 | ||
| result[(x < -2.5) & (x > -3.5)] = 13 | ||
| result[(x <= -3.5) & (x >= -5.0)] = 14 | ||
| result[x < -5.0] = 15 |
There was a problem hiding this comment.
The boundary conditions for quantization bins are inconsistent. Some bins are inclusive on both ends (e.g., [0.75, 1.25]), while others are exclusive (e.g., (0.25, 0.75)). This can lead to incorrect quantization for values that fall exactly on a boundary and inconsistent tie-breaking. For correctness and predictability, it's better to use a uniform convention for intervals, such as closed on the left and open on the right ([lower, upper)).
| result = torch.zeros_like(x, dtype=torch.uint8) | |
| result[(x >= 0.0) & (x <= 0.25)] = 0 | |
| result[(x > 0.25) & (x < 0.75)] = 1 | |
| result[(x >= 0.75) & (x <= 1.25)] = 2 | |
| result[(x > 1.25) & (x < 1.75)] = 3 | |
| result[(x >= 1.75) & (x <= 2.5)] = 4 | |
| result[(x > 2.5) & (x < 3.5)] = 5 | |
| result[(x >= 3.5) & (x <= 5.0)] = 6 | |
| result[x > 5.0] = 7 | |
| result[(x >= -0.25) & (x < -0.0)] = 8 | |
| result[(x < -0.25) & (x > -0.75)] = 9 | |
| result[(x <= -0.75) & (x >= -1.25)] = 10 | |
| result[(x < -1.25) & (x > -1.75)] = 11 | |
| result[(x <= -1.75) & (x >= -2.5)] = 12 | |
| result[(x < -2.5) & (x > -3.5)] = 13 | |
| result[(x <= -3.5) & (x >= -5.0)] = 14 | |
| result[x < -5.0] = 15 | |
| result = torch.zeros_like(x, dtype=torch.uint8) | |
| # Positive values | |
| result[(x >= 0.0) & (x < 0.25)] = 0 | |
| result[(x >= 0.25) & (x < 0.75)] = 1 | |
| result[(x >= 0.75) & (x < 1.25)] = 2 | |
| result[(x >= 1.25) & (x < 1.75)] = 3 | |
| result[(x >= 1.75) & (x < 2.5)] = 4 | |
| result[(x >= 2.5) & (x < 3.5)] = 5 | |
| result[(x >= 3.5) & (x < 5.0)] = 6 | |
| result[x >= 5.0] = 7 | |
| # Negative values | |
| result[(x >= -0.25) & (x < 0.0)] = 8 | |
| result[(x >= -0.75) & (x < -0.25)] = 9 | |
| result[(x >= -1.25) & (x < -0.75)] = 10 | |
| result[(x >= -1.75) & (x < -1.25)] = 11 | |
| result[(x >= -2.5) & (x < -1.75)] = 12 | |
| result[(x >= -3.5) & (x < -2.5)] = 13 | |
| result[(x >= -5.0) & (x < -3.5)] = 14 | |
| result[x < -5.0] = 15 |
| config_path = os.path.join(model_dir, "config.json") | ||
| if not os.path.exists(config_path): | ||
| raise ValueError("config.json is required to use --keep-last-n.") | ||
| cfg = json.load(open(config_path)) |
There was a problem hiding this comment.
It's a best practice to use a with statement when opening files. This ensures that the file is properly closed even if an exception occurs. This pattern of json.load(open(...)) or json.dump(..., open(...)) is used in a few places in this file (e.g., lines 408, 410, 418) and should be updated for robustness.
| cfg = json.load(open(config_path)) | |
| with open(config_path) as f: | |
| cfg = json.load(f) |
1 participant
@HumansAnd