Skip to content

Conversation

@tanmay17061
Copy link
Contributor

A bug in QdrantRM.forward signature implementation prevents it to be called without providing the argument k.

This was observed while using the dspy.ReAct module:

         (dspy/predict/react.py:95), in ReAct.act(self, output, hop)
     [92](dspy/predict/react.py:92)     return action_val
     [94](dspy/predict/react.py:94) try:
---> [95](dspy/predict/react.py:95)     output[f"Observation_{hop+1}"] = self.tools[action_name](action_val).passages
     [96](dspy/predict/react.py:96) except AttributeError:
     [97](dspy/predict/react.py:97)     # Handle the case where 'passages' attribute is missing
     [98](dspy/predict/react.py:98)     # TODO: This is a hacky way to handle this. Need to fix this.
     [99](dspy/predict/react.py:99)     output[f"Observation_{hop+1}"] = self.tools[action_name](action_val)

         (dspy/retrieve/retrieve.py:30), in Retrieve.__call__(self, *args, **kwargs)
     [29](dspy/retrieve/retrieve.py:29) def __call__(self, *args, **kwargs):
---> [30](dspy/retrieve/retrieve.py:30)     return self.forward(*args, **kwargs)

TypeError: QdrantRM.forward() missing 1 required positional argument: 'k'

@ammirsm
Copy link
Collaborator

ammirsm commented Apr 10, 2024

Should be pretty safe imo but I don't have setup to test it out, @arnavsinghvi11 do you have any input here or you think we can merge it?

@arnavsinghvi11 arnavsinghvi11 merged commit 539bdad into stanfordnlp:main Apr 11, 2024
@arnavsinghvi11
Copy link
Collaborator

merging. Thanks @tanmay17061 and @ammirsm !

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