Skip to content

MAINT: Renaming functions with a different name while dispatching #56

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.

Sign up for GitHub

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

Merged
dschult merged 2 commits into from
Apr 2, 2024
Merged

MAINT: Renaming functions with a different name while dispatching #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8694ba8
renamed approx_apnc and tour_isc
Schefflera-Arboricola Mar 29, 2024
3af1189
updated readme
Schefflera-Arboricola Mar 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,19 @@ nx-parallel is a NetworkX backend that uses joblib for parallelization. This pro
- [closeness_vitality](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/vitality.py#L9)
- [is_reachable](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/tournament.py#L10)
- [tournament_is_strongly_connected](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/tournament.py#L54)
- [all_pairs_node_connectivity](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/connectivity/connectivity.py#L17)
- [approximate_all_pairs_node_connectivity](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/approximation/connectivity.py#L12)
- [betweenness_centrality](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/centrality/betweenness.py#L16)
- [node_redundancy](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/bipartite/redundancy.py#L11)
- [all_pairs_bellman_ford_path](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/shortest_paths/weighted.py#L16)
- [johnson](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/shortest_paths/weighted.py#L59)
- [all_pairs_dijkstra](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/shortest_paths/weighted.py#L28)
- [all_pairs_dijkstra_path_length](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/shortest_paths/weighted.py#L71)
- [all_pairs_dijkstra_path](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/shortest_paths/weighted.py#L121)
- [all_pairs_bellman_ford_path_length](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/shortest_paths/weighted.py#L164)
- [all_pairs_bellman_ford_path](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/shortest_paths/weighted.py#L209)
- [johnson](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/shortest_paths/weighted.py#L252)
- [all_pairs_all_shortest_paths](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/shortest_paths/generic.py#L10)
- [all_pairs_shortest_path_length](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/shortest_paths/unweighted.py#L18)
- [all_pairs_shortest_path](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/algorithms/shortest_paths/unweighted.py#L62)

<details>
<summary>Script used to generate the above list</summary>
Expand Down Expand Up @@ -53,7 +62,7 @@ nxp.betweenness_centrality(H)

### Notes

1. Some functions in networkx have the same name but different implementations, so to avoid these name conflicts we differentiate them by the `name` parameter in `_dispatchable` at the time of dispatching (ref. [docs](https://networkx.org/documentation/latest/reference/generated/networkx.utils.backends._dispatchable.html#dispatchable)). So, mentioning either the full path of the implementation or the `name` parameter is recommended. For example:
1. Some functions in networkx have the same name but different implementations, so to avoid these name conflicts we differentiate them by the `name` parameter in `_dispatchable` at the time of dispatching (ref. [docs](https://networkx.org/documentation/latest/reference/generated/networkx.utils.backends._dispatchable.html#dispatchable)). So, `method 4` is not recommended. Instead, mentioning either the full path of the algorithm or the `name` parameter is recommended. For example:

```.py
# using full path
Expand Down
11 changes: 9 additions & 2 deletions nx_parallel/algorithms/approximation/connectivity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,25 @@
import nx_parallel as nxp

__all__ = [
"all_pairs_node_connectivity",
"approximate_all_pairs_node_connectivity",
Copy link
Member

@dschult dschult Apr 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If you want to allow both ways to call the functions, we could do something like

Suggested change
"approximate_all_pairs_node_connectivity",
"approximate_all_pairs_node_connectivity",
"all_pairs_node_connectivity",

and then below (after defining the longer named function) use

    all_pairs_node_connectivity = approximate_all_pairs_node_connectivity

But I'm fine with this PR as is. Which do you prefer?

Copy link
Member Author

@Schefflera-Arboricola Schefflera-Arboricola Apr 2, 2024

Choose a reason for hiding this comment

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

while generating the get_info function we fetch all the functions in the nx_parallel package and then extract their docstrings... so having 2 all_pairs_node_connectivity functions in the package(one of them being in connectivity/connectivity(with docstring) and the other one being in approximation/connectivity(without docstring)) would probably result in an incorrect mapping between functions and the additional_docs. So, for now, I'd prefer the current implemented way, although this way does allow the user to use nx-parallel as an independent package, so I'd like to try and find if there's a better way to generate get_info. But, I think that's for another PR.

]


def all_pairs_node_connectivity(G, nbunch=None, cutoff=None, get_chunks="chunks"):
def approximate_all_pairs_node_connectivity(
G, nbunch=None, cutoff=None, get_chunks="chunks"
):
"""The parallel implementation first divides the a list of all permutation (in case
of directed graphs) and combinations (in case of undirected graphs) of `nbunch`
into chunks and then creates a generator to lazily compute the local node
connectivities for each chunk, and then employs joblib's `Parallel` function to
execute these computations in parallel across all available CPU cores. At the end,
the results are aggregated into a single dictionary and returned.

Note, this function uses the name `approximate_all_pairs_node_connectivity` while
Copy link
Member Author

@Schefflera-Arboricola Schefflera-Arboricola Mar 29, 2024

Choose a reason for hiding this comment

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

also, added this to make the user aware that this function uses a different name while dispatching.

Copy link
Member

@dschult dschult Mar 29, 2024

Choose a reason for hiding this comment

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

I thought that users would never call this function directly -- that is, they would call the networkx function which in turn would call the appropriate function for them. Do I understand the public facing api correctly?

Copy link
Member Author

@Schefflera-Arboricola Schefflera-Arboricola Mar 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think we are that strict about how someone "should" use a backend and a backend is also a package. So, I added this for someone who might want to call nx_parallel's implementation directly for their graph(like if the cost of dispatching through networkx to nx-parallel is too much for their case). But, as mentioned in the note added in README(in this PR) it's not the recommended way. If you think it's too much info to put in the main docs, then let me know, I can remove it and just keep(or maybe change a little) the first note in README.

Copy link
Member

@dschult dschult Apr 2, 2024

Choose a reason for hiding this comment

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

OK... and I have a question: in the README file just after the paragraph describing this, the examples says:

 nx.approximate_all_pairs_node_connectivity(H)  # runs the parallel implementation in `approximation/connectivity`

But is this right? nx.approximate_all_pairs_node_connectivity(H) will raise due to the function not being found. In nx we call it as nx.approximation.all_pairs_node_connectivity(H)

I was thinking that the dispatcher calls the backend using the function nx_parallel.approximate_all_pairs_node_connectivity(H). but the user would still call it after importing networkx as nx.approximation.all_pairs_node_connectivity(H).

If I am right, we could allow both in the nx-parallel package (if you want to allow that) by including both routes to the name in the __init__.py files. But for now just having the longer name is Ok too. So I'm Ok with what is in this PR... But could you check the README.md example for how to call those functions?

Copy link
Member Author

@Schefflera-Arboricola Schefflera-Arboricola Apr 2, 2024

Choose a reason for hiding this comment

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

Thank you very much for pointing this out!
fixed in #58

dispatching to the backend in=mplementation. So, `nxp.all_pairs_node_connectivity`
will run the parallel implementation of `all_pairs_node_connectivity` present in the
`connectivity/connectivity`. Use `nxp.approximate_all_pairs_node_connectivity` instead.

Parameters
------------
get_chunks : str, function (default = "chunks")
Expand Down
8 changes: 6 additions & 2 deletions nx_parallel/algorithms/tournament.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

__all__ = [
"is_reachable",
"is_strongly_connected",
"tournament_is_strongly_connected",
]


Expand Down Expand Up @@ -51,11 +51,15 @@ def check_closure_subset(chunk):
return all(results)


def is_strongly_connected(G):
def tournament_is_strongly_connected(G):
"""The parallel computation is implemented by dividing the
nodes into chunks and then checking whether each node is reachable from each
other node in parallel.

Note, this function uses the name `tournament_is_strongly_connected` while
dispatching to the backend in=mplementation. So, `nxp.tournament.is_strongly_connected`
will result in an error. Use `nxp.tournament_is_strongly_connected` instead.

networkx.tournament.is_strongly_connected : https://networkx.org/documentation/stable/reference/algorithms/generated/networkx.algorithms.tournament.is_strongly_connected.html#networkx.algorithms.tournament.is_strongly_connected
"""
if hasattr(G, "graph_object"):
Expand Down
13 changes: 8 additions & 5 deletions nx_parallel/interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@
)
from nx_parallel.algorithms.efficiency_measures import local_efficiency
from nx_parallel.algorithms.isolate import number_of_isolates
from nx_parallel.algorithms import tournament
from nx_parallel.algorithms.tournament import (
is_reachable,
tournament_is_strongly_connected,
)
from nx_parallel.algorithms.vitality import closeness_vitality
from nx_parallel.algorithms.approximation.connectivity import (
all_pairs_node_connectivity,
approximate_all_pairs_node_connectivity,
)
from nx_parallel.algorithms.connectivity import connectivity
from nx_parallel.algorithms.cluster import square_clustering
Expand Down Expand Up @@ -55,8 +58,8 @@ class Dispatcher:
closeness_vitality = closeness_vitality

# Tournament
is_reachable = tournament.is_reachable
tournament_is_strongly_connected = tournament.is_strongly_connected
is_reachable = is_reachable
tournament_is_strongly_connected = tournament_is_strongly_connected

# Centrality
betweenness_centrality = betweenness_centrality
Expand All @@ -83,7 +86,7 @@ class Dispatcher:
all_pairs_shortest_path_length = all_pairs_shortest_path_length

# Approximation
approximate_all_pairs_node_connectivity = all_pairs_node_connectivity
approximate_all_pairs_node_connectivity = approximate_all_pairs_node_connectivity

# Connectivity
all_pairs_node_connectivity = connectivity.all_pairs_node_connectivity
Expand Down