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

Adds initial debug logging calls to _dispatchable #7300

Merged
merged 5 commits into from
May 29, 2024

Conversation

rlratzel
Copy link
Contributor

This PR adds debug logging for dispatched NetworkX functions.

import networkx as nx

G = nx.karate_club_graph()
result = nx.betweenness_centrality(G)

print(f"node with highest BC value is {sorted(result, key=lambda k: result[k])[-1]}")
bash$ NETWORKX_AUTOMATIC_BACKENDS=cugraph python example.py
node with highest BC value is 0

Debug logging is enabled using the standard python logging API:

import logging

import networkx as nx

logging.basicConfig(level=logging.DEBUG)

G = nx.karate_club_graph()
result = nx.betweenness_centrality(G)

print(f"node with highest BC value is {sorted(result, key=lambda k: result[k])[-1]}")
bash$ NETWORKX_AUTOMATIC_BACKENDS=cugraph python example_debug.py
DEBUG:networkx.utils.backends:using backend 'cugraph' for call to `betweenness_centrality' with args: (<nx_cugraph.classes.graph.Graph object at 0x7fc3dd53cb80>, None, True, None, False, <random.Random object at 0x56016e34f090>), kwargs: {}
node with highest BC value is 0

Debug messages are not shown when functions are not dispatched:

bash$ python example_debug.py
node with highest BC value is 0

@rossbar rossbar added type: Enhancements Dispatching Related to dispatching and backend support labels Feb 17, 2024
Copy link
Member

@dschult dschult left a comment

Choose a reason for hiding this comment

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

If adding debug logging will help with maintenance of this, its probably worth it. I typically use print statements for debugging and UserWarnings or exceptions for non-debugging "logging". But I don't use big complex software systems. So I'm approving this and will let more knowledgeable people decide how it is best set up.

@@ -523,6 +526,10 @@ def __call__(self, /, *args, backend=None, **kwargs):
fallback_to_nx=self._fallback_to_nx,
)
# All graphs are backend graphs--no need to convert!
_logger.debug(
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be good to have an env variable, like NETWORKX_BACKEND_DEBUG, and then only display these logging messages when this env variable is True.

Or use _logger.info instead of _logger.debug if you think this msg should be displayed to the user every time an algorithm is dispatched to a backend implementation.

also, what would happen if a backend implementation calls another backend implementation? I don't think that internally used function calls also get logged, but should they get logged? For eg, the below parallel implementation calls another parallel function(this is not actually the case, in nx-parallel all_pairs_bellman_ford_path calls the networkx's implementation of single_source_bellman_ford_path, i just wanted to give an eg):

def all_pairs_bellman_ford_path(G, weight="weight", get_chunks="chunks"):
   ....
   ....
    return [
            (node, nxp.single_source_bellman_ford_path(G, node, weight=weight))
            for node in node_chunk
        ]
    ....

so here the internally called function is single_source_bellman_ford_path and I'd suggest that this internally used function should also be logged. And the user can define the level(an env variable or something) of logging or debugging. LMK what u think.

Thank you :)

Copy link
Contributor

Choose a reason for hiding this comment

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

we can also add things like debug, caching, NETWORKX_AUTOMATIC_BACKENDS, and other backend env variables to networkx config.. this might also make it easier to implement @eriknw’s idea of bundling all the backend environment variables and ignoring them.

[Anthony] adding log levels(DEBUG, INFO, WARN, ERROR, etc.) (ref. docs)

Copy link
Member

@MridulS MridulS left a comment

Choose a reason for hiding this comment

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

Let's merge this in for now, we can reiterate on the logging bits in later PRs too.

@MridulS MridulS merged commit 2a03462 into networkx:main May 29, 2024
42 checks passed
@jarrodmillman jarrodmillman added this to the 3.4 milestone May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dispatching Related to dispatching and backend support type: Enhancements
Development

Successfully merging this pull request may close these issues.

None yet

6 participants