Skip to content

Conversation

teetone
Copy link
Member

@teetone teetone commented Dec 27, 2024

No description provided.

@teetone teetone requested review from yifanmai and ImKeTT December 27, 2024 07:11
Copy link
Collaborator

@ImKeTT ImKeTT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks all good to me!

@@ -191,6 +187,18 @@ def _make_chat_request(self, request: Request) -> RequestResult:
if raw_request["stop"] is None:
raw_request.pop("stop")

if request.model_engine == "o1-2024-12-17":
# Avoid error:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yifanmai note the extra logic for o1-2024-12-17 that's different from the other o1 models.

@@ -138,6 +138,13 @@ def alter_run_spec(run_spec: RunSpec) -> RunSpec:
):
run_spec = singleton(IncreaseMaxTokensRunExpander(value=1).expand(run_spec))

if model.name == "openai/o1-2024-12-17":
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yifanmai o1-2024-12-17 specifically is unusable in HELM without this change.

Copy link
Member Author

@teetone teetone Jan 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice, a few thousand reasoning tokens are used, at least for the VHELM scenarios.

@teetone teetone merged commit ee8cf38 into main Jan 5, 2025
8 checks passed
@teetone teetone deleted the newvlms branch January 5, 2025 05:12
@teetone teetone changed the title New QWEN 2 VLM New QWEN 2 VLM + o1 fixes for VHELM Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants