feat: add generative recommendation tokenizer. #317
Conversation
magicheng0816
requested review from
Clement-Wang26,
RobbieLeung,
walsonyang and
yq33victor
| std::lock_guard<std::mutex> lock(instance_map_mutex_); | ||
|
|
||
| auto it = instance_map_.find(version); | ||
| if (it == instance_map_.end()) { |
There was a problem hiding this comment.
why we need to find twice from instance_map_? Line25 above has already do this.
There was a problem hiding this comment.
double-checked locking, it is better to change std::mutex to std::shared_mutex
| } | ||
|
|
||
| bool RecVocabDict::get_items_by_tokens(const RecTokenTriple& rec_token_triple, | ||
| std::vector<int64_t>* item_ids) const { |
There was a problem hiding this comment.
nit: maybe use & here is better.
std::vector<int64_t>* item_ids -> const std::vector<int64_t>& item_ids
CHECK(!item_ids.empty());
There was a problem hiding this comment.
yes, the latter method is better, but the interface in front is the former method, so we used it
|
|
||
| /** | ||
| * @brief initialize instance, parse vocab file | ||
| * @param vocab_file vocab file, need full path |
There was a problem hiding this comment.
nit: maybe we can align the comment format.
// @brief initialize instance, parse vocab file
// @param vocab_file vocab file, need full path
// ...
| std::sort(model_weights_files_.begin(), model_weights_files_.end()); | ||
|
|
||
| //@todo: 'false' will be replaced with generative recommendation judgment | ||
| if (false) { |
There was a problem hiding this comment.
If it doesn't work yet, shouldn't add this logic for now.
There was a problem hiding this comment.
only send out the review in advance, will not merge now, and will be modified when merging the main process code of the generative recommendation.
3 participants
No description provided.