stanfordnlp / dspy

DSPy: The framework for programming—not prompting—foundation models
https://dspy-docs.vercel.app/
MIT License
17.99k stars 1.37k forks source link

Shadowing signature fields causes weird errors #658

Open thomasnormal opened 7 months ago

thomasnormal commented 7 months ago

I was just defining a signature like this:

class Signature(dspy.Signature):
    __doc__ = textwrap.dedent(
        """I gave my language model a task, but it failed. Figure out what went wrong,
        and write instructions to help it avoid the error next time.""",
    )

    task_description: str = dspy.InputField(desc="What I asked the model to do")
    model_output: str = dspy.InputField(desc="The output of the model")
    error: str = dspy.InputField(desc="The validation error trigged by the models output")
    explanation: str = dspy.OutputField(desc="Explain what the model did wrong")
    instructions: str = dspy.OutputField(desc="Instructions for the model to do better next time")

but this fails with

    raise TypeError(
TypeError: Field 'instructions' in 'Signature' must be declared with InputField or OutputField. field.json_schema_extra=None

Why? Because instructions is a "reserved word" of dspy.Signature.

There are a number of other reserved words, like fields, insert, prepend, append, equals. At least a few of those have already caused me trouble in the past. On top of that there are "pydantic reserved words", like schema that I've also had issues with.

Pydantic 2.x is trying to clean this up, by moving all their special words to the prefix model_, such as model_schema instead of schema. Just now I made a field with name model_output and got this warning from pydantic: "...pydantic/_internal/_fields.py:149: UserWarning: Field "model_output" has conflict with protected namespace "model_"

I think we should similarly move all the dspy related words to some prefix, like dspy_instructions, dspy_equals etc. This causes only a small inconvenience for dspy backend developers (e.g. you are making a new optimizer), but make life a lot easier for end users (that is, people just defining a casual signature.)

On top of that, we should make sure to throw informative errors in the cases where there's still a clash. Like if the user uses model_schema or dspy_instructions as a field name for some reason.

thomasnormal commented 7 months ago

An alternative idea is to move everything out of the Signature namespace and into the module. Then you would do dspy.signature.instructions(sig) instead of sig.dspy_instructions.

thomasnormal commented 7 months ago

A related issue is "reserved keywords" for dspy.Predict. The forward method assigns special meaning to words like new_signature, signature, demos, config, lm, past_outputs and _trace. So if a user uses any of those words as names in a Signature, they'll probably also run into trouble.

Even if we decide to do nothing in terms of cleaning up the namespace; we should at least make sure we document those reserved names well, and throw errors when the user unknowingly tries to use them.

okhat commented 7 months ago

Ah this is a good point. Moving things out of the name space makes sense. Maybe we can reserve just one word, e.g. sig._metadata

cognitivetech commented 7 months ago

inputs is another reserved keyword

thomasahle commented 5 months ago

@arnavsinghvi11 I don't think anything was done to fix this problem, no? Please correct me if I'm wrong. Otherwise I'll keep it open to help us keep track of the problem.