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

Prompt changed based on when dspy.settings.configure(lm=lm) is executed. #1589

Open
mikeedjones opened this issue Oct 6, 2024 · 3 comments

Comments

@mikeedjones
Copy link
Contributor

mikeedjones commented Oct 6, 2024

If you configure the LM (dspy.settings.configure(lm=lm)) before initializing your modules you get a different prompt to if you configure your LM after initializing your modules.

    dspy.settings.configure(lm=lm)
    pot = ChainOfThought(BasicQA)

Adds "reasoning" to the output signature.

    pot = ChainOfThought(BasicQA)
    dspy.settings.configure(lm=lm)

Adds "rationale" to the output signature.

Think its because the dspy.settings.lm is referenced in ChainOfThought.__init__:

        # Add "rationale" field to the output signature.
        if isinstance(dspy.settings.lm, dspy.LM):
            extended_signature = signature.prepend("reasoning", rationale_type, type_=str)
        else:
            extended_signature = signature.prepend("rationale", rationale_type, type_=str)

Maybe something to flag in migration docs or just to be aware of before this logic is removed when dsp.LM is removed.

@okhat
Copy link
Collaborator

okhat commented Oct 6, 2024

Very interesting, thanks for the catch @mikeedjones ! Good reason to migrate fully in 2.6 soon

@okhat
Copy link
Collaborator

okhat commented Oct 9, 2024

Since this could be fairly annoying, I'm thinking the following: if we don't release 2.6 (which deletes old clients) within a very short time, we should make them only work if dspy.configure(legacy=True) before any DSPy usage. Once legacy is turned on, it cannot be turned off, i.e. you cannot mix and match 2.5 and 2.4 style DSPy.

That way, we can greatly simplify the if statement causing this issue.

@mikeedjones
Copy link
Contributor Author

mikeedjones commented Oct 9, 2024

As far as I can tell this change only affects ChainOfThought and Predict - could there be a ChainOfThoughtv2 or something for the change over period?

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

2 participants