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

Why does the Executor.Call function not pass the opts parameter to Agent.Plan? #353

Open
tyloafer opened this issue Nov 12, 2023 · 3 comments

Comments

@tyloafer
Copy link

When I follow the test to use agent

func TestMRKL(t *testing.T) {
	t.Parallel()

	if openaiKey := os.Getenv("OPENAI_API_KEY"); openaiKey == "" {
		t.Skip("OPENAI_API_KEY not set")
	}
	if serpapiKey := os.Getenv("SERPAPI_API_KEY"); serpapiKey == "" {
		t.Skip("SERPAPI_API_KEY not set")
	}

	llm, err := openai.New()
	require.NoError(t, err)

	searchTool, err := serpapi.New()
	require.NoError(t, err)

	calculator := tools.Calculator{}

	a, err := agents.Initialize(
		llm,
		[]tools.Tool{searchTool, calculator},
		agents.ZeroShotReactDescription,
	)
	require.NoError(t, err)

        result, err := chains.Run(context.Background(), a, "If a person lived three times as long as Jacklyn Zeman, how long would they live",
		chains.WithTemperature(1),
		chains.WithTopK(1),
		chains.WithTopP(1)) //nolint:lll
	require.NoError(t, err)

	require.True(t, strings.Contains(result, "210"), "correct answer 210 not in response")
}

I find that i cannot paas param like StopWords/MaxTokens to openai llm

when i follow the code, i find that the Executor.Call function not pass the opts parameter to Agent.Plan
which causes that the param i have set is useless

unc (e Executor) Call(ctx context.Context, inputValues map[string]any, _ ...chains.ChainCallOption) (map[string]any, error) { //nolint:lll
	inputs, err := inputsToString(inputValues)
	if err != nil {
		return nil, err
	}
	nameToTool := getNameToTool(e.Tools)

	steps := make([]schema.AgentStep, 0)
	for i := 0; i < e.MaxIterations; i++ {
		var finish map[string]any
		steps, finish, err = e.doIteration(ctx, steps, nameToTool, inputs)
		if finish != nil || err != nil {
			return finish, err
		}
	}

	return e.getReturn(
		&schema.AgentFinish{ReturnValues: make(map[string]any)},
		steps,
	), ErrNotFinished
}

func (e Executor) doIteration(
	ctx context.Context,
	steps []schema.AgentStep,
	nameToTool map[string]tools.Tool,
	inputs map[string]string,
) ([]schema.AgentStep, map[string]any, error) {
	actions, finish, err := e.Agent.Plan(ctx, steps, inputs)

So, Why does the Executor.Call function not pass the opts parameter to Agent.Plan? Is there some reason?

@tmc
Copy link
Owner

tmc commented Mar 19, 2024

I agree that options should be threaded through to Plan. I have #463 filed and I think this is an example of something that should be revamped.

Given that upstream is planning/prepping for 0.2.0, I think some new design thinking around not just the interface to the agents package but implementation details like this should be fixed. Do you want to propose a change to add variadic options to Plan() and pass it through here?

@tyloafer
Copy link
Author

Yes, in my local reference, I have appended variadic parameters to Plan(). And I agree with your point, I am looking forward to the release of version 0.2.0.

disk0Dancer added a commit to disk0Dancer/langchaingo that referenced this issue May 23, 2024
disk0Dancer added a commit to disk0Dancer/langchaingo that referenced this issue May 25, 2024
disk0Dancer added a commit to disk0Dancer/langchaingo that referenced this issue May 25, 2024
@wukai123123123
Copy link

Yes, passing is invalid when I use chains.WithTemperature(0.95)

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

No branches or pull requests

3 participants