Skip to content
This repository was archived by the owner on Jul 16, 2025. It is now read-only.

Add topP support #37

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/OpenAI/Model/Gpt.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,31 @@
use PhpLlm\LlmChain\Response\Choice;
use PhpLlm\LlmChain\Response\Response;
use PhpLlm\LlmChain\Response\ToolCall;
use Webmozart\Assert\Assert;

final class Gpt implements LanguageModel
{
public function __construct(
private readonly Runtime $runtime,
private ?Version $version = null,
private readonly float $temperature = 1.0,
private readonly float $topP = 1.0,
Comment on lines 21 to +22
Copy link
Member

Choose a reason for hiding this comment

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

What do you think in general of having those parameters here? Struggle a bit with my initial decision - should maybe only the part of the $options of call(..) method?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think for default settings for fine tuning this position is fitting. But maybe all settings available should be put to a config object?

When i have a look to the OpenAI Playground ... there are more settings beside temperature and topP for every request. So this would stack up to this amount.

image

Maybe this could also be utilized as argument by the user?

Copy link
Member

Choose a reason for hiding this comment

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

maintaining a config object would be hard - the api contract is growing and depends on versions and platform. we could tackle that of course with having designated extension points, but having an array is way more flexible for now 🙊 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can go with $options array then, but we couldn't validate them, just pass them to the model -> fire and forget... 🤷‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

The model/Platform will validate anyways and we're just reproducing their rules here

Copy link
Member

Choose a reason for hiding this comment

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

We need way better error handling tho for http calls. Was quite lazy with that ... 👼

) {
$this->version ??= Version::gpt4o();

Assert::greaterThanEq($temperature, 0.01);
Copy link
Member

Choose a reason for hiding this comment

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

the api also allows 0

Suggested change
Assert::greaterThanEq($temperature, 0.01);
Assert::greaterThanEq($temperature, 0);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on my research it needs to be at least 0.01

Copy link
Member

Choose a reason for hiding this comment

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

Research meaning usage? Pretty sure i used 0 before but can check tomorrow or later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CleanShot 2024-09-25 at 22 09 11@2x

Yes you are right, based on the docs, however, when I played with the assistants:
CleanShot 2024-09-25 at 22 10 33@2x

You cannot go below 0.01

Copy link
Member

Choose a reason for hiding this comment

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

UI says no, API says yes

image

image

Assert::lessThanEq($temperature, 2);

Assert::greaterThanEq($topP, 0.01);
Assert::lessThanEq($topP, 1);
}

public function call(MessageBag $messages, array $options = []): Response
{
$body = [
'model' => $this->version->name,
'temperature' => $this->temperature,
'top_p' => $this->topP,
'messages' => $messages,
];

Expand Down