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

Remove restoring token limit on opening settings #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jks-liu
Copy link
Contributor

@jks-liu jks-liu commented Oct 16, 2024

  1. Behavior of setting limit to a default value every time of opening settings is very strange.
  2. The default value is not suitable for num_ctx parameter because it is the maximum supported value by model. For e.g. llama3.1, this value is more than 100k, passing it to num_ctx will decrease performance. (Default value for ollama is only 2048)

1. Behavior of setting limit to a default value every time of opening settings is very strange.
2. The default value is not suitable for num_ctx parameter because it is the maximum supported value by model.
For e.g. llama3.1, this value is more than 100k, passing it to num_ctx will decrease performance. (Default value for ollama is only 2048)
@tcsenpai
Copy link
Owner

Do you mean that in your PR we don't use the tokens table anymore and we just rely on the user's settings (or default)?

Regarding (2) I did not find any drawbacks on this, can you provide an example of why it would be detrimental to set num_ctx to 100k for example?

@jks-liu
Copy link
Contributor Author

jks-liu commented Oct 17, 2024

  1. Yes, I think token limit should be decided by users because users have different hardware.
  2. On my RTX 4090, llama3.1 will be slow if num_ctx is more than 10k.

More discussion about num_ctx:
ollama/ollama#4790
Mintplex-Labs/anything-llm#1991
https://www.reddit.com/r/LocalLLaMA/comments/1dxi6cf/today_i_learned_the_context_length_num_ctx/

@tcsenpai
Copy link
Owner

Do you think it is still worth making the (2) edit now that I merged the other PR?

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