Skip to content

Add should_run functionality#123

Merged
dschult merged 18 commits intonetworkx:mainfrom
akshitasure12:should_run
Aug 19, 2025
Merged

Add should_run functionality#123
dschult merged 18 commits intonetworkx:mainfrom
akshitasure12:should_run

Conversation

@akshitasure12
Copy link
Contributor

@akshitasure12 akshitasure12 commented Jul 17, 2025

This PR handles #77.

  • I ran the following as a test:
NETWORKX_BACKEND_PRIORITY_ALGOS=parallel NETWORKX_FALLBACK_TO_NX=False python3 test_should_run.py

Contents of test_should_run.py :

import networkx as nx
import logging

nxl = logging.getLogger("networkx")
nxl.addHandler(logging.StreamHandler())
nxl.setLevel(logging.DEBUG)

G = nx.empty_graph(5)
print(nx.number_of_isolates(G))

Output:

Call to 'number_of_isolates' has inputs from {'networkx'} backends, and will try to use backends in the following order: ['parallel', 'networkx']
Backend 'parallel' shouldn't run `number_of_isolates` with arguments: (G=<networkx.classes.graph.Graph object at 0x1055fb6e0>), because: Fast algorithm; not worth converting
Trying next backend: 'networkx'
Call to 'isolates' has inputs from {'networkx'} backends, and will try to use backends in the following order: ['parallel', 'networkx']
Backend 'parallel' does not implement 'isolates'
Trying next backend: 'networkx'
5

@Schefflera-Arboricola Schefflera-Arboricola added the type: Enhancement New feature or request label Jul 17, 2025
Copy link
Member

@Schefflera-Arboricola Schefflera-Arboricola left a comment

Choose a reason for hiding this comment

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

Consider adding some unit tests for should_run-- creating a centralised test-- like test_get_chunks-- might be good here.

Copy link
Member

@Schefflera-Arboricola Schefflera-Arboricola left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @akshitasure12 !

Left a few comments/suggestions below. LMKWYT.

Also, could we add should_run for more functions* with similar should run messages and have a single should_run-- instead of having same should_run for several algorithms? LMK if that's possible. Thanks!

*some shortest paths functions also doesn't show much speed ups-- refer timing folder for more

@akshitasure12
Copy link
Contributor Author

akshitasure12 commented Jul 28, 2025

Also, could we add should_run for more functions* with similar should run messages and have a single should_run-- instead of having same should_run for several algorithms? LMK if that's possible. Thanks!

I’ve added should_run as an argument to _configure_if_nx_active() and moved the various should_run helpers for different graph conditions (like number of nodes) into a separate file named should_run_policies.py(open to renaming if you have suggestions). This eliminates the need to create individual _ functions for different algorithms. Let me know what you think!

I tested it on test_should_run.py:

import networkx as nx
import logging

nxl = logging.getLogger("networkx")
nxl.addHandler(logging.StreamHandler())
nxl.setLevel(logging.DEBUG)

G = nx.empty_graph(5)
print(nx.number_of_isolates(G))

Output:

Call to 'number_of_isolates' has inputs from {'networkx'} backends, and will try to use backends in the following order: ['parallel', 'networkx']
Backend 'parallel' shouldn't run `number_of_isolates` with arguments: (G=<networkx.classes.graph.Graph object at 0x103f9d6a0>), because: Fast algorithm; skip parallel execution
Trying next backend: 'networkx'
Call to 'isolates' has inputs from {'networkx'} backends, and will try to use backends in the following order: ['parallel', 'networkx']
Backend 'parallel' does not implement 'isolates'
Trying next backend: 'networkx'
5

Copy link
Member

@Schefflera-Arboricola Schefflera-Arboricola left a comment

Choose a reason for hiding this comment

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

review comments from meeting:
- including should_run for if n_jobs=None or 1 or 0 for all nx-parallel algorithms-- default should run?
- Adding should_run to more algorithms-- based on performance heatmaps' trends

@akshitasure12 akshitasure12 changed the title Add should_run functionality [WIP]: Add should_run functionality Aug 13, 2025
@akshitasure12 akshitasure12 changed the title [WIP]: Add should_run functionality Add should_run functionality Aug 15, 2025
@akshitasure12
Copy link
Contributor Author

@dschult The should_run functionality has been implemented, and the tests are passing. It’s ready for review-- looking forward to the feedback.
Thank you :)

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.

This looks good!
I have one suggestion to remove the word "custom" in one place (to match the other removals). If leaving it was intentional then leave it. :)

Co-authored-by: Dan Schult <dschult@colgate.edu>
@dschult dschult merged commit d4377b3 into networkx:main Aug 19, 2025
8 checks passed
@dschult dschult added this to the 0.4 milestone Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: Enhancement New feature or request

Development

Successfully merging this pull request may close these issues.

3 participants