Empty assert hunt #6056
Empty assert hunt #6056
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6056 +/- ##
==========================================
- Coverage 78.82% 78.47% -0.36%
==========================================
Files 146 146
Lines 26200 26204 +4
==========================================
- Hits 20653 20564 -89
- Misses 5547 5640 +93
Continue to review full report at Codecov.
|
|
I'm in love 😍 😍. Thanks @TevenLeScao ! |
Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
| self.state_dict = dict(self.state_dict) | ||
| self.wemb, self.final_bias = add_emb_entries(self.state_dict["Wemb"], self.state_dict[BIAS_KEY], 1) | ||
| self.pad_token_id = self.wemb.shape[0] - 1 | ||
| cfg["vocab_size"] = self.pad_token_id + 1 | ||
| # self.state_dict['Wemb'].sha | ||
| self.state_keys = list(self.state_dict.keys()) | ||
| if "Wtype" in self.state_dict: | ||
| raise ValueError("found Wtype key") | ||
| assert "Wtype" not in self.state_dict, "Wtype key in state dictionary" |
There was a problem hiding this comment.
Are you sure ? it seems to me that the assert clause activates when the key is in the state dict, not the reverse, so that's what the error message should display
There was a problem hiding this comment.
no you're correct. I don't see the value of the change but my suggestion is horrible.
There was a problem hiding this comment.
Yeah I was just trying to harmonize things, not much of a change from raising an error to having an assert clause
Co-authored-by: Sam Shleifer <sshleifer@gmail.com>
| self.state_dict = dict(self.state_dict) | ||
| self.wemb, self.final_bias = add_emb_entries(self.state_dict["Wemb"], self.state_dict[BIAS_KEY], 1) | ||
| self.pad_token_id = self.wemb.shape[0] - 1 | ||
| cfg["vocab_size"] = self.pad_token_id + 1 | ||
| # self.state_dict['Wemb'].sha | ||
| self.state_keys = list(self.state_dict.keys()) | ||
| if "Wtype" in self.state_dict: | ||
| raise ValueError("found Wtype key") | ||
| assert "Wtype" not in self.state_dict, "Wtype key in state dictionary" |
There was a problem hiding this comment.
Are you sure ? it seems to me that the assert clause activates when the key is in the state dict, not the reverse, so that's what the error message should display
|
@LysandreJik can I merge this? |
* Fixed empty asserts * black-reformatted stragglers in templates * More code quality checks * Update src/transformers/convert_marian_to_pytorch.py Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com> * Update src/transformers/convert_marian_to_pytorch.py Co-authored-by: Sam Shleifer <sshleifer@gmail.com> * removed unused line as per @sshleifer Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com> Co-authored-by: Sam Shleifer <sshleifer@gmail.com>
Empty asserts are bad for debugging. I tried to remove them all and to add helpful Pytorch-style messages with the shapes of the corresponding objects when they were mismatched-lengths checks + the file paths when they were file-not-found checks.