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

Started implementation of temperature, top_k and logits for local models #31

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

Conversation

karemr2024
Copy link

  • Removed hardcoded temperature, topk and logits
  • Modified GeminiExecutor implementation to initialize temperature, topk and logits
  • Modified atomic.rs config struct to include temperature, topk and logits
  • Added the default values in case GeminiExecutor is initialized without new arguments
  • Added test for new version of GeminiExecutor

@andthattoo
Copy link
Owner

Thanks for the contribution!

We can't merge this unless the newly added

    pub temperature: Option<f64>,   // Add temperature field
    pub top_k: Option<i32>,         // Add top_k field
    pub logits: Option<bool>, 

fields to Config struct are usable by each provider including OpenAI, Gemini, OpenRouter and Ollama.

Also logits are not supported by Ollama yet (see issue)

Only option is to work vLLM out for compute nodes so logits are supported. Maybe we can handle temp and top_k for all providers and keep them in the struct but remove logits until we have vLLM support.

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