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

GCM auto assignment now returns a summary object #1049

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

bloebp
Copy link
Member

@bloebp bloebp commented Oct 16, 2023

The summary object contains information about the evaluated models and model choices. This object is printable to provide quick summary.

@bloebp bloebp requested a review from kailashbuki October 16, 2023 19:40
@bloebp bloebp force-pushed the gcm_auto_assign_verbose branch 4 times, most recently from c77a1ae to 96e6ff0 Compare October 17, 2023 23:01
dowhy/gcm/config.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@kailashbuki kailashbuki left a comment

Choose a reason for hiding this comment

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

My recommendation would be to use the global logger directly in functions where you need verbosity, and control the verbosity of the logger instead from the argument. Example that leverages a decorator to modify log level in a function where you need verbosity:

First, I will set the default log level to DEBUG.

@set_log_verbosity
def my_func(arg1, kwarg1=v1, verbosity=False):
     log.info('my verbose message')

Here the set_log_verbosity decorator function will modify the log level to INFO if verbosity is set to True, return the wrapped function, and reset the verbosity to DEBUG again.

@bloebp
Copy link
Member Author

bloebp commented Oct 18, 2023

My recommendation would be to use the global logger directly in functions where you need verbosity, and control the verbosity of the logger instead from the argument. Example that leverages a decorator to modify log level in a function where you need verbosity:

First, I will set the default log level to DEBUG.

@set_log_verbosity
def my_func(arg1, kwarg1=v1, verbosity=False):
     log.info('my verbose message')

Here the set_log_verbosity decorator function will modify the log level to INFO if verbosity is set to True, return the wrapped function, and reset the verbosity to DEBUG again.

The problem is, I want that these messages appear by default and not if a user sets a logger (which 99% of users probably won't). On the other hand, I also don't want to give it out as a print message, i.e., I set the loggign level to info and only add the messages if verbose is true.

Do you have another suggestions where a user does not need to put in additional work to get these insights?

@kailashbuki
Copy link
Collaborator

kailashbuki commented Oct 18, 2023

My recommendation would be to use the global logger directly in functions where you need verbosity, and control the verbosity of the logger instead from the argument. Example that leverages a decorator to modify log level in a function where you need verbosity:
First, I will set the default log level to DEBUG.

@set_log_verbosity
def my_func(arg1, kwarg1=v1, verbosity=False):
     log.info('my verbose message')

Here the set_log_verbosity decorator function will modify the log level to INFO if verbosity is set to True, return the wrapped function, and reset the verbosity to DEBUG again.

The problem is, I want that these messages appear by default and not if a user sets a logger (which 99% of users probably won't). On the other hand, I also don't want to give it out as a print message, i.e., I set the loggign level to info and only add the messages if verbose is true.

Do you have another suggestions where a user does not need to put in additional work to get these insights?

This is what I mean roughly (there is of course a wiggle room for how to want to parse the verbosity arg):

def set_log_verbosity(func):
    def wrapper(*args, **kwargs):
        if kwargs.get('verbosity', None):
           # change log level to desired level
        res = func(*args, **kwargs)
        if kwargs.get('verbosity', None):
            # reset log level to default level
        return res
    return wrapper

@bloebp bloebp force-pushed the gcm_auto_assign_verbose branch from 96e6ff0 to 076aa77 Compare October 19, 2023 22:35
@bloebp bloebp changed the title Add verbose parameter to GCM auto assign function GCM auto assignment now returns a summary object Oct 19, 2023
@bloebp
Copy link
Member Author

bloebp commented Oct 19, 2023

I revised the logic and the auto assignment now returns a summary object instead which the user can print. This is also helpful to have a more programmatic access to the evaluated models and their performances.

@bloebp bloebp force-pushed the gcm_auto_assign_verbose branch from 076aa77 to 9814e41 Compare October 19, 2023 22:38
dowhy/gcm/auto.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@kailashbuki kailashbuki left a comment

Choose a reason for hiding this comment

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

Unit tests are missing for the change.

The summary object contains information about the evaluated models and model choices. This object is printable to provide quick summary.

Signed-off-by: Patrick Bloebaum <bloebp@amazon.com>
@bloebp bloebp force-pushed the gcm_auto_assign_verbose branch from 9814e41 to 1540584 Compare October 20, 2023 15:54
@bloebp bloebp merged commit d83fc3f into main Nov 2, 2023
30 checks passed
@bloebp bloebp deleted the gcm_auto_assign_verbose branch November 2, 2023 18:42
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