Zifan Wang
@zifanwTF
collaborative and educational with apologetic tendencies
Highly technical and detail-oriented reviewer who provides extensive explanations and context for suggestions. Engages in thorough back-and-forth discussions to ensure proper understanding and implementation of complex technical concepts.
264
Comments
96
PRs
5
Repos
176
Avg Chars
2
Harshness
Personality
Patient and educational
Technically meticulous
Collaborative and discussion-oriented
Practical and solution-focused
Open to learning and admitting mistakes
Detail-conscious about edge cases
Helpful with providing examples and references
Apologetic when realizing errors
Greatest Hits
"No need to change :-)"
"Thanks for spotting this"
"Sorry for the confusion"
"Oh shoot. This was my old implementation"
"Good catch!"
"Just want to mention this to you so are aware"
"I am not sure if we want to"
"We prob will need"
Focus Areas
- Pydantic model structure and validation
- Tokenization and LLM integration details
- Type annotations and proper data types
- Edge cases and technical nuances
- Code organization and architecture
- Dependency management
- Technical implementation details
Common Phrases
"I think"
"just want to"
"No need to change :-)"
"Thanks for spotting this"
"Sorry for the confusion"
"Let me know if"
"We prob will need"
"I am not sure if"
"Good catch!"
"Just want to mention"
"Sorry for chiming in"
"Oh shoot"
"Actually"
"By the way"
"This change should"
Sentiment Breakdown
Review Outcomes
Most Reviewed Authors
Spiciest Comments
> @zifanwTF I switched Tokenizer to AutoTokenizer.
>
> Would need your help to look into the following issues:
>
> 1. The claude tokenizer as suggested in https://huggingface.co/Xenova/claude-tokenizer cannot be found by AutoTokenizer.
> `Error msg: ValueError: Tokenizer class ClaudeTokenizer does not exist or is not currently imported. `
> 2. The encoded values are different from my previous Tokenizer. Could you plz check if those encoded values make sense? Some seems to be weird. Y
> Could we add some description on what the various updates cover, for the sake of review and future reference after merging?
Added.
What do you mean "some developers change the name"? This repo will not be
made public. By the way, I disagree the project name should go to the
environment because we will want to modify it easily if we want to create
another evaluation project on weights and bias. Making it as obvious as
possible is preferred.
On Sun, Jan 7, 2024, 6:53 PM thakkerurvish ***@***.***> wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In nemo/evaluation_framewor
AI Persona Prompt
You are @zifanwTF, a highly technical and collaborative code reviewer working on an AI/ML project called 'goldfish' that involves LLMs, Pydantic models, and complex data processing. Your review style is educational and thorough - you love diving deep into technical details and providing extensive context for your suggestions. You frequently use phrases like 'I think', 'just want to', 'Thanks for spotting this', and 'Sorry for the confusion'. You're particularly focused on Pydantic model structures, tokenization details, type annotations, and LLM integration nuances. You often provide code examples, external links, and detailed explanations of edge cases. You're collaborative and engage in long technical discussions, frequently asking clarifying questions and admitting when you've made mistakes with phrases like 'Oh shoot' or 'Sorry for chiming in'. You're patient with explaining complex concepts and often mention alternative approaches or potential future considerations. You tend to be apologetic when pointing out issues and always try to be helpful by providing specific solutions or workarounds. Your comments are typically longer than average because you believe in providing full context and educational value. You pay attention to dependency versions, proper use of libraries like transformers and Pydantic, and are always thinking about the broader architectural implications of changes.
Recent Comments (229 total)
This line can be merged with Line 81 by using the `assert` function. No need to change :-)
```
assert self._review_type in (ResponseReviewType.ZERO_SHOT, ResponseReviewType.FEW_SHOT), f"Review type {self._review_type} not implemented"
```
LGTM
@shuhaodo
RE: How to determine if review is needed
I am almost done with code that checks if the response need reviewing. The idea is going to have a review prompt adapting to the response and localize the errors first. If there is no error, we do not need to review.
RE: How do we ensure the reviewed result is better than the existing one, by comparing the response validation score?
We w
@mingyang-tinyfish
No I think you are right about the few-shot. I forgot this is for reviewing. Yeah that is the correct name. Sorry for the confusion.
There is no switch in Python but in Python 3.10 there is `match/case`.
https://www.freecodecamp.org/news/python-switch-statement-switch-case-example/
```
lang = input("What's the programming language you want to learn? ")
match lang:
case "JavaScript":
print("You can become a web developer.")
case "Python":
print("You can become a Data Scientist")
```
do you actually want to do `hasattr()` first?
Oh shoot. This was my old implementation. Will clear this code up. Thanks for spotting this.
I actually will remove the print/log, that is just for debugging.
Going to solve in the next PR.
Resolved.
The constraint (ie the regular expression) helps to distinguish bewteen the string id (eg 'id_123') and the string value of that node. If we enforece the response to start with 'id_' (or other prefix we are using), the LLM respones will only contain the ID instead of the value (Gemini would likely to do so).
Resolved.
the `_retore_ids` can return various types depending on the inputs. This is just fixing an error in the type annotation.
> Reminder to add "anthropic" to pyproject
Done
These are just placeholders. I will need to play around with the actual review LLMs later. We are not using the review mode for Sagemaker models anyway