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

Improve error logging with a stacktrace #1458

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

severi
Copy link

@severi severi commented Sep 6, 2024

Related to issue #788

This prints out the example inputs and a stacktrace for the error that occurred. I understand that this change would add more excessive error messages that you have tried to avoid

The current behavior was designed to limit excessive outputted error messages, partly handled through how many errors can occur before completely halting execution

But at least to me this has been the number 1 issue I have been experiencing when testing out dspy for the past week. This change works like a charm in my use case.

If the printed error message seems too excessive with large datasets, then another option would be to add the stacktrace logging behind an env variable that can be set if needed (though my personal preference would be to have the stacktrace visible as the default option and having the possibility to limit the error traces with a similar approach)

 if os.environ.get('DEBUG') == 'true':
     dspy.logger.error(f"Error for example in dev set: \t\t {e}\n\twith inputs:\n\t\t{example.inputs()}\n\nStack trace:\n\t{traceback.format_exc()}")

@severi severi mentioned this pull request Sep 6, 2024
@krypticmouse
Copy link
Collaborator

Hi @severi

Thanks for the contribution! Is it possible to make this configurable?

I think we can keep a variable to define how verbose the traceback should be, something like:

class Evaluate:
    def __init__(
        self,
        *,
        devset,
        metric=None,
        num_threads=1,
        display_progress=False,
        display_table=False,
        max_errors=5,
        return_all_scores=False,
        return_outputs=False,
        provide_traceback = False
        **_kwargs,
    ):
        self.devset = devset
        ...
        self.return_outputs = return_outputs
        self.provide_traceback = provide_traceback
        
    def evaluate(...):
        ....
        if self.provide_traceback:
            dspy.logger.error(f"Error for example in dev set: \t\t {e}\n\twith inputs:\n\t\t{example.inputs()}\n\nStack trace:\n\t{traceback.format_exc()}")
        else:
            dspy.logger.error(f"Error for example in dev set: \t\t {e}")

@krypticmouse
Copy link
Collaborator

Also can you resolve the conflicts? I think is just import merging from 2 branches that's all 🙂

Copy link
Collaborator

@krypticmouse krypticmouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some changes needed but mostly looks good!

@severi
Copy link
Author

severi commented Sep 19, 2024

Hey! Implemented the requested changes + fixed the merge conflict

@severi
Copy link
Author

severi commented Sep 19, 2024

We could also modify the original error message to something like

dspy.logger.error(f"Error for example in dev set: \t\t {e}. Set `provide_traceback=True` to see the stack trace.")

to make it more visible such an option exists

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.

2 participants