Skip to content

Conversation

@mikeusru
Copy link

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,
            }

@drawal1
Copy link
Contributor

drawal1 commented Apr 26, 2024

Reviewed. I fixed this comprehensively in #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

@arnavsinghvi11
Copy link
Collaborator

@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

@drawal1
Copy link
Contributor

drawal1 commented Jun 3, 2024

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

arnavsinghvi11 added a commit that referenced this pull request Jun 19, 2024
Fixed issue #894 and #858 (aws_models issues)
@arnavsinghvi11
Copy link
Collaborator

Resolved by merged #843

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

Successfully merging this pull request may close these issues.

3 participants