Skip to content

Conversation

@xyloid
Copy link
Contributor

@xyloid xyloid commented May 1, 2024

This problem is very similar to #769. OllamaLocal.copy uses LM.copy. However, OllamaLocal's kwargs dict doesn't have model key, which means model = kwargs.pop("model") will create a KeyError. This fix added a copy method to OllamaLocal class so that it's own attributes like model, model_type, base_url and timeout_s can be copied.

@arnavsinghvi11
Copy link
Collaborator

Hi @xyloid , thanks for the contribution! Could you please rebase and merge to the latest version on main? Seems like there was a failing test on our end (nothing wrong with this PR) that just needs an update. Ready to merge after that!

@xyloid xyloid force-pushed the fix/ollama_copy branch from 91a82d4 to c7d45b6 Compare May 4, 2024 23:20
@xyloid
Copy link
Contributor Author

xyloid commented May 4, 2024

Hi @xyloid , thanks for the contribution! Could you please rebase and merge to the latest version on main? Seems like there was a failing test on our end (nothing wrong with this PR) that just needs an update. Ready to merge after that!

Hi @arnavsinghvi11 , I have rebased my branch on the latest main branch.

@arnavsinghvi11 arnavsinghvi11 merged commit 7b76165 into stanfordnlp:main May 5, 2024
@arnavsinghvi11
Copy link
Collaborator

Thanks @xyloid !

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