fix(basemodel): Format AssertionError message for max_seq_length vs max_total_token_num#1300
fix(basemodel): Format AssertionError message for max_seq_length vs max_total_token_num#1300hiworldwzj merged 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a validation check in the _check_mem_size method of the base model to ensure that max_total_token_num is greater than or equal to max_seq_length, except when the performance mode is set to 'personal'. Feedback from the review suggests improving the consistency of error messages by including actual values in the existing batch_max_tokens assertion and correcting a logical discrepancy and grammar in the newly added assertion message.
| assert ( | ||
| self.max_total_token_num > self.batch_max_tokens | ||
| ), "max_total_token_num must be greater than batch_max_tokens" |
There was a problem hiding this comment.
For consistency with the detailed error message added below, consider including the actual values of max_total_token_num and batch_max_tokens in this assertion message. This helps users diagnose configuration issues more easily.
| assert ( | |
| self.max_total_token_num > self.batch_max_tokens | |
| ), "max_total_token_num must be greater than batch_max_tokens" | |
| assert self.max_total_token_num > self.batch_max_tokens, ( | |
| f"max_total_token_num must be > batch_max_tokens, " | |
| f"got max_total_token_num={self.max_total_token_num}, " | |
| f"batch_max_tokens={self.batch_max_tokens}. " | |
| f"Try setting --batch_max_tokens to a smaller value < {self.max_total_token_num}." | |
| ) |
| assert self.max_seq_length <= self.max_total_token_num, ( | ||
| f"max_total_token_num must be >= max_seq_length, " | ||
| f"got max_total_token_num={self.max_total_token_num}, " | ||
| f"max_seq_length={self.max_seq_length}. " | ||
| f"Try set --max_req_total_len a smaller value < {self.max_total_token_num}." | ||
| ) |
There was a problem hiding this comment.
The error message suggestion uses < but the condition is <=. Since max_seq_length can be equal to max_total_token_num, the suggestion should reflect that. Also, 'Try set' should be 'Try setting' for better grammar.
| assert self.max_seq_length <= self.max_total_token_num, ( | |
| f"max_total_token_num must be >= max_seq_length, " | |
| f"got max_total_token_num={self.max_total_token_num}, " | |
| f"max_seq_length={self.max_seq_length}. " | |
| f"Try set --max_req_total_len a smaller value < {self.max_total_token_num}." | |
| ) | |
| assert self.max_seq_length <= self.max_total_token_num, ( | |
| f"max_total_token_num must be >= max_seq_length, " | |
| f"got max_total_token_num={self.max_total_token_num}, " | |
| f"max_seq_length={self.max_seq_length}. " | |
| f"Try setting --max_req_total_len to a value <= {self.max_total_token_num}." | |
| ) |
No description provided.