dspy icon indicating copy to clipboard operation
dspy copied to clipboard

fix(dspy/modules/aws_models): properly copy kwargs so temporary changes don't propagate to base model

Open mikeusru opened this issue 10 months ago • 1 comments

When the kwargs of the model get altered for the body, we don't want the new kwargs saved to the original llm object. This fixes a bug caused by the mutability of kwargs.

In my case, the bug resulted in the max_tokens to repeatedly get cut in half every time i would re-run the model due to following logic in predict.py

            max_tokens = min(max(75, max_tokens // 2), max_tokens)
            keys = list(kwargs.keys()) + list(dsp.settings.lm.kwargs.keys()) 
            max_tokens_key = "max_tokens" if "max_tokens" in keys else "max_output_tokens"
            new_kwargs = {
                **kwargs,
                max_tokens_key: max_tokens,
                "n": 1,
                "temperature": 0.0,
            }

mikeusru avatar Apr 17 '24 19:04 mikeusru

Reviewed. I fixed this comprehensively in https://github.com/stanfordnlp/dspy/pull/843. You caught the dict copy issue but there's another check that's also needed. You can review the aws_models.py in my PR

drawal1 avatar Apr 26 '24 22:04 drawal1

@drawal1 - could we potentially isolate that patch and push it within this PR? Seems like #843 is more involved and needs some conflicts to be resolved before merging

arnavsinghvi11 avatar May 31 '24 04:05 arnavsinghvi11

@arnavsinghvi11 - I reverted all the complicated changes in PR #843 so that can be merged and this PR can be closed

drawal1 avatar Jun 03 '24 15:06 drawal1

Resolved by merged #843

arnavsinghvi11 avatar Jun 19 '24 22:06 arnavsinghvi11