stanfordnlp / dspy

DSPy: The framework for programming—not prompting—language models
https://dspy.ai
MIT License
18.84k stars 1.44k forks source link

MIPROv2 - Support for -> None in __init__ Method Signature #1476

Closed williambrach closed 1 month ago

williambrach commented 2 months ago

The current regex pattern used to extract the init method content doesn't account for return type annotations, specifically -> None. This causes an error when processing classes with annotated init methods.

Current Behavior

The following code snippet causes an error:

class MyModule(dspy.Module):
    def __init__(self) -> None:
        super().__init__()
        self.prog = dspy.ChainOfThought(MySignature)

Error message:

grounded_proposer.py:197, in GenerateModuleInstruction.forward(self, demo_candidates, pred_i, demo_set_i, program, previous_instructions, data_summary, max_demos, tip)
    195 init_pattern = r"def __init__\(.*?\):([\s\S]*?)(?=^\s*def|\Z)"
    196 init_content_match = re.search(init_pattern, self.program_code_string)
--> 197 init_content = init_content_match.group(0)
    198 pattern = r"^(.*dspy\.(ChainOfThought|Predict).*)$"  # TODO: make it so that this extends out to any dspy Module
    199 matches = re.findall(pattern, init_content, re.MULTILINE)

AttributeError: 'NoneType' object has no attribute 'group'

Workaround

Removing -> None from the init method signature allows MIPROv2 to work:

class MyModule(dspy.Module):
    def __init__(self):
        super().__init__()
        self.prog = dspy.ChainOfThought(MySignature)

Proposed Solution

Update the regex pattern to optionally match return type annotations:

init_pattern = r"def __init__\(.*?\)(?: -> None)?:([\s\S]*?)(?=^\s*def|\Z)"

This pattern should match init methods both with and without -> None annotations.

I can create PR if solution my is suitable :)

okhat commented 2 months ago

Thank you @williambrach ! Yes a PR is welcome! Though this may suggest we need a more fundamental solution to this cc: @XenonMolecule

williambrach commented 2 months ago

Thank you @okhat for asap response. Should I wait for @XenonMolecule to discuss potential solution (e.g updating regex to support any string not only None) or I should create PR with "None" fix?

Thanks!

okhat commented 2 months ago

Thanks @williambrach ! I'd want to wait for @XenonMolecule to consider what's a fundamental fix here because we don't just want to keep making small patches to MIPRO.

To make sure you're unblocked, @williambrach , perhaps you need to just remove the type annotation for now :D

williambrach commented 2 months ago

@okhat don't worry. I completed of run of my DSPy experiments without -> None annotation

okhat commented 1 month ago

fixed