Clean up unused codes and update README #8
Conversation
| ```bash | ||
| export ANTHROPIC_API_KEY=secret_abcdefg | ||
| export FURIOSA_API_BASE=FURIOSA_API_ENDOPINT |
There was a problem hiding this comment.
| export FURIOSA_API_BASE=FURIOSA_API_ENDOPINT | |
| export FURIOSA_API_BASE=FURIOSA_API_ENDPOINT |
그런데 예시의 목적이라면 OPENAI_API_BASE 와 같이 http://127.0.0.1:8000/v1 같은 것이 직관적이지 않을까 싶습니다.
| --results-dir "result_outputs" \ | ||
| --llm-api anthropic \ | ||
| --llm-api furiosa \ |
There was a problem hiding this comment.
furiosa_client는 사실상 openai client와 큰 차이는 없지만 일부 (불필요한) 환경변수나 설정을 wrapping하는 용도로 보이는데 이해한게 맞을까요?
언뜻 보기에는 --llm-api openai 와 큰 차이가 없어보여서 어떤 부분에서 다른지 설명이 있으면 좋을 것 같습니다.
There was a problem hiding this comment.
맞습니다. OPENAI_*와 같은 환경변수 대신 FURIOSA_* 환경변수를 사용하도록 하기 위함이 큰데요, open ai api와 호환된다면 furiosa client는 없애도 될까요? 어떤 방식으로 사용하는게 더 적절할지 잘 모르겠어서 의견을 여쭙고 싶습니다.
리뷰주신 다른 부분들도 이 부분이 정리되면 함께 정리될 수 있을 것 같습니다.
| import ray | ||
| from ray.runtime_env import RuntimeEnv | ||
|
|
||
|
|
||
| FURIOSA_AI_DEFAULT_SYSTEM_PROMPT = "You are a helpful assistant." |
There was a problem hiding this comment.
| FURIOSA_AI_DEFAULT_SYSTEM_PROMPT = "You are a helpful assistant." | |
| DEFAULT_SYSTEM_PROMPT = "You are a helpful assistant." |
Furiosa만의 특별한 prompt는 아닌 것 같아, 단순히 DEFAULT_SYSTEM_PROMPT 라고만 표기해도 충분하지 않을까 싶습니다.
| if "temperature" not in request_config.sampling_params: | ||
| request_config.sampling_params["temperature"] = 0.0 | ||
| request_config.system_prompt = FURIOSA_AI_DEFAULT_SYSTEM_PROMPT |
There was a problem hiding this comment.
모든 request에 대한 system prompt가 default로 고정되는 것 같습니다.
None 인 경우에만 기본값을 적용하도록 해야할 것 같습니다.
또는 models.py의 RequestConfig에 기본값을 명시하는 것도 방법이 될 것 같습니다.
| @@ -16,6 +16,7 @@ class RequestConfig(BaseModel): | |||
|
|
|||
| model: str | |||
| prompt: Tuple[str, int] | |||
| system_prompt: Optional[str] = None | |||
There was a problem hiding this comment.
docstring에 system_prompt 항목을 추가해주면 좋을 것 같습니다.
| python benchmark.py \ | ||
| --model "meta-llama/Llama-3.2-1B-Instruct" \ | ||
| --dataset translation \ | ||
| --mean-input-tokens 550 \ | ||
| --stddev-input-tokens 150 \ | ||
| --mean-output-tokens 150 \ | ||
| --stddev-output-tokens 10 \ | ||
| --max-num-completed-requests 2 \ | ||
| --timeout 600 \ | ||
| --num-concurrent-requests 1 \ | ||
| --wait-for any \ | ||
| --results-dir "result_outputs" \ | ||
| --llm-api anthropic \ | ||
| --llm-api furiosa \ | ||
| --additional-sampling-params '{}' |
There was a problem hiding this comment.
README를 통해 usage와 args 를 알 수 있게끔 하는 것이 목적이라면,
각 항목들에 대한 설명이 추가되면 많은 도움이 될 것 같습니다.
가장 간단하게는 required에 해당하는 값만 넣고 실행할 때의 동작과 optional argument를 지정했을 때의 효과에 대해 기술되면 좋을 것 같습니다.
내용이 많아질 수 있으므로, 틀만 만들어두고 차차 채워나가는 것도 괜찮을 것 같습니다.
There was a problem hiding this comment.
네 README에 각 옵션에 대한 설명은 차차 채워나가겠습니다.
사용하지 않는 코드들을 제거하고, 코드 위치를 조금 옮기고, README를 업데이트 했습니다.