Be more specific about missign RoPE parameters #1781
Conversation
| @@ -129,8 +129,9 @@ def rope_cache(self, device: Optional[torch.device] = None) -> Tuple[torch.Tenso | |||
| } | |||
| else: | |||
| # Some but not all parameters are specified; raise an error | |||
| missing_params = [param for param, present in zip(adjusted_params_required, params_present) if not present] | |||
There was a problem hiding this comment.
I thought that zip in case of iterators of unequal length will iterate till the shortest one.
Maybe you wanted to use itertools.zip_longest, but then how can you guarantee that items in the list will match.
Perhaps convert to sets and return difference.
There was a problem hiding this comment.
Or you can rewrite the code a bit.
Iterate over required keys and if it is in the config - add it to a dict. If not - to the list of missing keys.
There was a problem hiding this comment.
So weird, I tested it yesterday and could swear it worked. But looking at it again with fresh eyes, I do agree with your point about zip.
There was a problem hiding this comment.
Ok relooking at it again, I don't see an issue @Andrei-Aksionov since both lists have the same length in zip. Note that params_present is always a list that has the same length as adjusted_params_required:
adjusted_params_required = ["factor", "low_freq_factor", "high_freq_factor", "original_max_seq_len"]
params_present = [param in self.config.rope_adjustments for param in adjusted_params_required]
There was a problem hiding this comment.
Again 🙂
Yes, you are right. It's a list of booleans and not a list of keys, so the length should be the same.
It's just more of a mask, rather than a list of params that are presented in the config. That what confused me.
Suggests which parameters are missing if some of the new extra RoPE parameters are used (as suggested by @Andrei-Aksionov )