Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Avoid empty keyword argument in VLLMModelConfig from Makefile #246

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mattdepaolis
Copy link

Issue Summary
When running the make evaluate command with the specified parameters, an error occurs indicating that the VLLMModelConfig initializer received an unexpected keyword argument.

Steps to Reproduce:

  1. Set up the environment as per the project's guidelines.
  2. Execute the following command:
make evaluate MODEL=deepseek-ai/DeepSeek-R1-Distill-Qwen-1.5B TASK=math_500
  1. Expected Behavior: The evaluation process should complete without errors, utilizing the specified model and task parameters.

  2. Actual Behavior: The following error message is displayed:

TypeError: VLLMModelConfig.__init__() got an unexpected keyword argument ''

Root Cause Analysis

  • The MODEL_ARGS variable in the Makefile's evaluate target includes an empty $(PARALLEL_ARGS), which introduces consecutive commas.
  • This results in an empty string being passed as a keyword argument to VLLMModelConfig.

Proposed Solution
Modify the MODEL_ARGS assignment in the Makefile to conditionally include $(PARALLEL_ARGS) only if it's not empty:

MODEL_ARGS="pretrained=$(MODEL),dtype=bfloat16$(if $(PARALLEL_ARGS),,$(PARALLEL_ARGS)),max_model_length=32768,gpu_memory_utilisation=0.8"

This prevents the introduction of an unintended empty string argument.

Testing

  • Ran the make evaluate command with and without PARALLEL_ARGS and verified that no unexpected keyword argument error occurs.

Copy link

@Charlesnorris509 Charlesnorris509 left a comment

Choose a reason for hiding this comment

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

@mattdepaolis This PR seems to be solid and follow best practices, however The evaluate target is quite complex and involves multiple nested conditionals and shell commands. This will be difficult to maintain.

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.

Error in make evaluate: TypeError: VLLMModelConfig.__init__() got an unexpected keyword argument ''
2 participants