(dspy/predict): corrected temperature check for bedrock claude models

[mikeusru]

PR linked to issue described here:
#795 (comment)

Using Bedrock and claude3-haiku

bedrock = dspy.Bedrock(...)

haiku = dspy.AWSAnthropic(
    aws_provider=bedrock,
    model="anthropic.claude-3-haiku-20240307-v1:0",
)

class BasicQA(dspy.Signature):
    """Answer questions with short factoid answers."""

    question = dspy.InputField()
    answer = dspy.OutputField(desc="often between 1 and 5 words")
    
# Define the predictor. Notice we're just changing the class. The signature BasicQA is unchanged.
generate_answer_with_chain_of_thought = dspy.ChainOfThought(BasicQA)

# Call the predictor on the same input.
pred = generate_answer_with_chain_of_thought(question=dev_example.question)

Error: TypeError: '>' not supported between instances of 'NoneType' and 'int'

The error is fixed when the change in the PR is introduced

[arnavsinghvi11]

Thanks for raising this @mikeusru ! This looks like a global change not specific to AWS models, but I'm curious if it is fixed when you specify a default n in AWSModels as 1 like other models do. I see that num_generations is not supported but if n is, that should fix this issue? tagging @drawal1 for AWSModels.

[drawal1]

@arnavsinghvi11 - I have an alternative (and hopefully more robust) suggestion:

Instead of:
num_generations = lm.kwargs.get("n", lm.kwargs.get("num_generations", None))

How about?
num_generations = lm.kwargs.get("n", lm.kwargs.get("num_generations", 1))

[mikeusru]

@arnavsinghvi11 - I have an alternative (and hopefully more robust) suggestion:

Instead of: num_generations = lm.kwargs.get("n", lm.kwargs.get("num_generations", None))

How about? num_generations = lm.kwargs.get("n", lm.kwargs.get("num_generations", 1))

I believe "n" causes errors as well, and needs to be removed from the kwargs for Claude 3 models. I do see Bedrock documentation about batch inference in preview, but i haven't gotten this to work.
https://docs.aws.amazon.com/bedrock/latest/userguide/batch-inference.html

[arnavsinghvi11]

hmm yeah so ideally, we’d want n to be set in either the config or the configured LM and merging this would bypass that if either is not set. do none of the AWSModels support n or any alternatives?

[drawal1]

@arnavsinghvi11 @mikeusru - 'n' is a non-issue. I just verified that 'n' is NOT inserted by default in query_args for anthropic.

If somebody deliberately introduces it and bedrock/anthropic complains, then its a valid complaint. We don't want to patch for dev mistakes. Or am I missing something?

[arnavsinghvi11]

makes sense. just wanted to confirm batching behavior for AWS models. adding in the default behavior for num_generations and merging. Thanks @mikeusru @drawal1 !