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

Duplicate GeminiAdapter class definition found #3462

Open
KangmoonSeo opened this issue Aug 3, 2024 · 0 comments · May be fixed by #3463
Open

Duplicate GeminiAdapter class definition found #3462

KangmoonSeo opened this issue Aug 3, 2024 · 0 comments · May be fixed by #3463

Comments

@KangmoonSeo
Copy link

I've noticed that there are two identical class definitions for GeminiAdapter in the same file. This appears to be an unintended duplication.

  • File: fastchat/model/model_adapter.py
  • Lines: 1202-1212 and 2193-2205
# fastchat/model/model_adapter.py

# line 1202
class GeminiAdapter(BaseModelAdapter):
    """The model adapter for Gemini"""

    def match(self, model_path: str):
        return "gemini" in model_path.lower() or "bard" in model_path.lower()

    def load_model(self, model_path: str, from_pretrained_kwargs: dict):
        raise NotImplementedError()

    def get_default_conv_template(self, model_path: str) -> Conversation:
        return get_conv_template("gemini")


# line 2193
class GeminiAdapter(BaseModelAdapter):
    """The model adapter for Gemini"""

    def match(self, model_path: str):
        return "gemini" in model_path.lower() or "bard" in model_path.lower()

    def load_model(self, model_path: str, from_pretrained_kwargs: dict):
        raise NotImplementedError()

    def get_default_conv_template(self, model_path: str) -> Conversation:
        if "gemini-1.5-pro" in model_path:
            return get_conv_template("gemini-1.5-pro")
        return get_conv_template("gemini")

The classes are identical except for the get_default_conv_template method. The second definition includes an additional check for "gemini-1.5-pro".

Suggestion:
Consider merging these two class definitions, keeping the more specific get_default_conv_template method from the second definition. This would avoid confusion and potential issues with class resolution.

Please let me know if you need any further information or clarification.

@KangmoonSeo KangmoonSeo linked a pull request Aug 3, 2024 that will close this issue
3 tasks
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 a pull request may close this issue.

1 participant