From 21a43459a7694740937a0cff0278f6e26ed0e0b0 Mon Sep 17 00:00:00 2001 From: Aditi Juneja <91629733+Schefflera-Arboricola@users.noreply.github.com> Date: Fri, 20 Dec 2024 06:54:53 +0530 Subject: [PATCH 1/7] Update checklist (#90) --- CONTRIBUTING.md | 9 +++++---- nx_parallel/tests/test_get_chunks.py | 28 +++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8d538c34..79ce6d56 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -121,11 +121,12 @@ The default chunking in nx-parallel is done by slicing the list of nodes (or edg - The algorithm that you are considering to add to nx-parallel should be in the main networkx repository and it should have the `_dispatchable` decorator. If not, you can consider adding a sequential implementation in networkx first. - check-list for adding a new function: - [ ] Add the parallel implementation(make sure API doesn't break), the file structure should be the same as that in networkx. - - [ ] add the function to the `BackendInterface` class in [interface.py](./nx_parallel/interface.py) (take care of the `name` parameter in `_dispatchable` (ref. [docs](https://networkx.org/documentation/latest/reference/backends.html))) + - [ ] Include the `get_chunks` additional parameter. Currently, all algorithms in nx-parallel offer the user to pass their own custom chunks. Unless it is impossible to chunk, please do include this additional parameter. + - [ ] add the function to the `ALGORITHMS` list in [interface.py](./nx_parallel/interface.py). Take care of the `name` parameter in `_dispatchable` for the algorithms with same name but different implementations. The `name` parameter is used distinguish such algorithms in a single namespace. (ref. [docs](https://networkx.org/documentation/latest/reference/backends.html))) - [ ] update the `__init__.py` files accordingly - [ ] docstring following the above format - - [ ] run the [timing script](./timing/timing_individual_function.py) to get the performance heatmap - - [ ] add additional test(if any) - - [ ] add benchmark(s) for the new function(ref. the README in benchmarks folder for more details) + - [ ] add additional test, if needed. The smoke tests for the additional parameter `get_chunks` are done [here](https://github.com/networkx/nx-parallel/blob/main/nx_parallel/tests/test_get_chunks.py) together for all the algorithms. + - [ ] run the [timing script](./timing/timing_individual_function.py) to get the performance heatmap (ref. [Issue#51](https://github.com/networkx/nx-parallel/issues/51)) + - [ ] add benchmark(s) for the new function(ref. the README in `benchmarks` folder for more details) Happy contributing! 🎉 diff --git a/nx_parallel/tests/test_get_chunks.py b/nx_parallel/tests/test_get_chunks.py index c8e63f9e..38255e07 100644 --- a/nx_parallel/tests/test_get_chunks.py +++ b/nx_parallel/tests/test_get_chunks.py @@ -22,7 +22,21 @@ def get_all_functions(package_name="nx_parallel"): for name, obj in inspect.getmembers(package, inspect.isfunction): if not name.startswith("_"): - args, kwargs = inspect.getfullargspec(obj)[:2] + signature = inspect.signature(obj) + args = [ + param.name + for param in signature.parameters.values() + if param.kind + in ( + inspect.Parameter.POSITIONAL_ONLY, + inspect.Parameter.POSITIONAL_OR_KEYWORD, + ) + ] + kwargs = [ + param.name + for param in signature.parameters.values() + if param.kind == inspect.Parameter.KEYWORD_ONLY + ] functions[name] = {"args": args, "kwargs": kwargs} return functions @@ -76,14 +90,18 @@ def random_chunking(nodes): if isinstance(c1, types.GeneratorType): c1, c2 = dict(c1), dict(c2) if func in chk_dict_vals: - for i in range(len(G.nodes)): - assert math.isclose(c1[i], c2[i], abs_tol=1e-16) + for edge in G.edges: + assert math.isclose( + c1.get(edge, 0), c2.get(edge, 0), abs_tol=1e-16 + ) else: assert c1 == c2 else: if func in chk_dict_vals: - for i in range(len(G.nodes)): - assert math.isclose(c1[i], c2[i], abs_tol=1e-16) + for edge in G.edges: + assert math.isclose( + c1.get(edge, 0), c2.get(edge, 0), abs_tol=1e-16 + ) else: if isinstance(c1, float): assert math.isclose(c1, c2, abs_tol=1e-16) From 8d1fc5993a40309fbd70531dca7e607739d44e6e Mon Sep 17 00:00:00 2001 From: Akshita Sure Date: Mon, 7 Apr 2025 00:41:19 +0530 Subject: [PATCH 2/7] Refactored tests and function logic --- nx_parallel/tests/test_get_chunks.py | 32 ++++++++++------------------ 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/nx_parallel/tests/test_get_chunks.py b/nx_parallel/tests/test_get_chunks.py index 38255e07..387e8f3a 100644 --- a/nx_parallel/tests/test_get_chunks.py +++ b/nx_parallel/tests/test_get_chunks.py @@ -18,36 +18,26 @@ def get_all_functions(package_name="nx_parallel"): the function's keyword arguments and positional arguments. """ package = importlib.import_module(package_name) - functions = {} + all_funcs_kwargs = {} for name, obj in inspect.getmembers(package, inspect.isfunction): if not name.startswith("_"): signature = inspect.signature(obj) - args = [ - param.name - for param in signature.parameters.values() - if param.kind - in ( - inspect.Parameter.POSITIONAL_ONLY, - inspect.Parameter.POSITIONAL_OR_KEYWORD, - ) - ] kwargs = [ param.name for param in signature.parameters.values() - if param.kind == inspect.Parameter.KEYWORD_ONLY + if param.default is not inspect.Parameter.empty ] - functions[name] = {"args": args, "kwargs": kwargs} - - return functions + all_funcs_kwargs[name] = kwargs + return all_funcs_kwargs def get_functions_with_get_chunks(): """Returns a list of function names with the `get_chunks` kwarg.""" - all_funcs = get_all_functions() + all_funcs_kwargs = get_all_functions() get_chunks_funcs = [] - for func in all_funcs: - if "get_chunks" in all_funcs[func]["args"]: + for func in all_funcs_kwargs: + if "get_chunks" in all_funcs_kwargs[func]: get_chunks_funcs.append(func) return get_chunks_funcs @@ -90,17 +80,17 @@ def random_chunking(nodes): if isinstance(c1, types.GeneratorType): c1, c2 = dict(c1), dict(c2) if func in chk_dict_vals: - for edge in G.edges: + for key in c1.keys(): assert math.isclose( - c1.get(edge, 0), c2.get(edge, 0), abs_tol=1e-16 + c1.get(key, 0), c2.get(key, 0), abs_tol=1e-16 ) else: assert c1 == c2 else: if func in chk_dict_vals: - for edge in G.edges: + for key in c1.keys(): assert math.isclose( - c1.get(edge, 0), c2.get(edge, 0), abs_tol=1e-16 + c1.get(key, 0), c2.get(key, 0), abs_tol=1e-16 ) else: if isinstance(c1, float): From f1c93dba38d278118b8b31ce5e80e109d479eef9 Mon Sep 17 00:00:00 2001 From: Akshita Sure Date: Sat, 3 May 2025 15:07:34 +0530 Subject: [PATCH 3/7] Improved error handling in test_get_chunks() and fixed get_all_functions_kwargs() to return kwargs --- nx_parallel/tests/test_get_chunks.py | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/nx_parallel/tests/test_get_chunks.py b/nx_parallel/tests/test_get_chunks.py index 387e8f3a..d8958c5e 100644 --- a/nx_parallel/tests/test_get_chunks.py +++ b/nx_parallel/tests/test_get_chunks.py @@ -10,12 +10,12 @@ import nx_parallel as nxp -def get_all_functions(package_name="nx_parallel"): - """Returns a dict keyed by function names to its arguments. +def get_all_functions_kwargs(package_name="nx_parallel"): + """Returns a dict keyed by function names to its positional-or-keyword arguments. This function constructs a dictionary keyed by the function names in the package `package_name` to dictionaries containing - the function's keyword arguments and positional arguments. + the function's positional-or-keyword arguments. """ package = importlib.import_module(package_name) all_funcs_kwargs = {} @@ -26,7 +26,7 @@ def get_all_functions(package_name="nx_parallel"): kwargs = [ param.name for param in signature.parameters.values() - if param.default is not inspect.Parameter.empty + if param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD ] all_funcs_kwargs[name] = kwargs return all_funcs_kwargs @@ -34,7 +34,7 @@ def get_all_functions(package_name="nx_parallel"): def get_functions_with_get_chunks(): """Returns a list of function names with the `get_chunks` kwarg.""" - all_funcs_kwargs = get_all_functions() + all_funcs_kwargs = get_all_functions_kwargs() get_chunks_funcs = [] for func in all_funcs_kwargs: if "get_chunks" in all_funcs_kwargs[func]: @@ -80,18 +80,20 @@ def random_chunking(nodes): if isinstance(c1, types.GeneratorType): c1, c2 = dict(c1), dict(c2) if func in chk_dict_vals: - for key in c1.keys(): - assert math.isclose( - c1.get(key, 0), c2.get(key, 0), abs_tol=1e-16 - ) + for key in c1: + if not math.isclose(c1[key], c2[key], abs_tol=1e-16): + raise ValueError( + f"Values for key '{key}' differ: {c1[key]} != {c2[key]}" + ) else: assert c1 == c2 else: if func in chk_dict_vals: for key in c1.keys(): - assert math.isclose( - c1.get(key, 0), c2.get(key, 0), abs_tol=1e-16 - ) + if not math.isclose(c1[key], c2[key], abs_tol=1e-16): + raise ValueError( + f"Values for key '{key}' differ: {c1[key]} != {c2[key]}" + ) else: if isinstance(c1, float): assert math.isclose(c1, c2, abs_tol=1e-16) From 480cabf39edb31ea923e0453141db11791e3a6ff Mon Sep 17 00:00:00 2001 From: Akshita Sure Date: Sat, 3 May 2025 15:20:16 +0530 Subject: [PATCH 4/7] Replaced c1.keys() with c1 --- nx_parallel/tests/test_get_chunks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nx_parallel/tests/test_get_chunks.py b/nx_parallel/tests/test_get_chunks.py index d8958c5e..459ad695 100644 --- a/nx_parallel/tests/test_get_chunks.py +++ b/nx_parallel/tests/test_get_chunks.py @@ -89,7 +89,7 @@ def random_chunking(nodes): assert c1 == c2 else: if func in chk_dict_vals: - for key in c1.keys(): + for key in c1: if not math.isclose(c1[key], c2[key], abs_tol=1e-16): raise ValueError( f"Values for key '{key}' differ: {c1[key]} != {c2[key]}" From 4d57ce338cd982f7ea584373d6f8a87f7c26f5c6 Mon Sep 17 00:00:00 2001 From: Akshita Sure Date: Sat, 10 May 2025 13:27:40 +0530 Subject: [PATCH 5/7] Updated the naming --- nx_parallel/tests/test_get_chunks.py | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/nx_parallel/tests/test_get_chunks.py b/nx_parallel/tests/test_get_chunks.py index 459ad695..f6fd402e 100644 --- a/nx_parallel/tests/test_get_chunks.py +++ b/nx_parallel/tests/test_get_chunks.py @@ -10,31 +10,31 @@ import nx_parallel as nxp -def get_all_functions_kwargs(package_name="nx_parallel"): - """Returns a dict keyed by function names to its positional-or-keyword arguments. +def get_all_function_arguments(package_name="nx_parallel"): + """Returns a dict keyed by function names to its arguments. This function constructs a dictionary keyed by the function names in the package `package_name` to dictionaries containing - the function's positional-or-keyword arguments. + the function's arguments. """ package = importlib.import_module(package_name) - all_funcs_kwargs = {} + all_funcs_arguments = {} for name, obj in inspect.getmembers(package, inspect.isfunction): if not name.startswith("_"): signature = inspect.signature(obj) - kwargs = [ + arguments = [ param.name for param in signature.parameters.values() if param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD ] - all_funcs_kwargs[name] = kwargs - return all_funcs_kwargs + all_funcs_arguments[name] = arguments + return all_funcs_arguments def get_functions_with_get_chunks(): """Returns a list of function names with the `get_chunks` kwarg.""" - all_funcs_kwargs = get_all_functions_kwargs() + all_funcs_kwargs = get_all_function_arguments() get_chunks_funcs = [] for func in all_funcs_kwargs: if "get_chunks" in all_funcs_kwargs[func]: @@ -81,19 +81,13 @@ def random_chunking(nodes): c1, c2 = dict(c1), dict(c2) if func in chk_dict_vals: for key in c1: - if not math.isclose(c1[key], c2[key], abs_tol=1e-16): - raise ValueError( - f"Values for key '{key}' differ: {c1[key]} != {c2[key]}" - ) + assert math.isclose(c1[key], c2[key], abs_tol=1e-16) else: assert c1 == c2 else: if func in chk_dict_vals: for key in c1: - if not math.isclose(c1[key], c2[key], abs_tol=1e-16): - raise ValueError( - f"Values for key '{key}' differ: {c1[key]} != {c2[key]}" - ) + assert math.isclose(c1[key], c2[key], abs_tol=1e-16) else: if isinstance(c1, float): assert math.isclose(c1, c2, abs_tol=1e-16) From e496d455f6dceb4c619ddbb79d4465b83944d03e Mon Sep 17 00:00:00 2001 From: Aditi Juneja <91629733+Schefflera-Arboricola@users.noreply.github.com> Date: Sun, 11 May 2025 00:43:26 +0530 Subject: [PATCH 6/7] a few final touches --- nx_parallel/tests/test_get_chunks.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/nx_parallel/tests/test_get_chunks.py b/nx_parallel/tests/test_get_chunks.py index f6fd402e..4fba5a54 100644 --- a/nx_parallel/tests/test_get_chunks.py +++ b/nx_parallel/tests/test_get_chunks.py @@ -10,15 +10,13 @@ import nx_parallel as nxp -def get_all_function_arguments(package_name="nx_parallel"): - """Returns a dict keyed by function names to its arguments. - - This function constructs a dictionary keyed by the function - names in the package `package_name` to dictionaries containing - the function's arguments. +def get_all_funcs_with_args(package_name="nx_parallel"): + """Returns a dict keyed by function names to a list of + the function's args names, for all the functions in + the package `package_name`. """ package = importlib.import_module(package_name) - all_funcs_arguments = {} + funcs_with_args = {} for name, obj in inspect.getmembers(package, inspect.isfunction): if not name.startswith("_"): @@ -28,16 +26,16 @@ def get_all_function_arguments(package_name="nx_parallel"): for param in signature.parameters.values() if param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD ] - all_funcs_arguments[name] = arguments - return all_funcs_arguments + funcs_with_args[name] = arguments + return funcs_with_args def get_functions_with_get_chunks(): """Returns a list of function names with the `get_chunks` kwarg.""" - all_funcs_kwargs = get_all_function_arguments() + all_funcs = get_all_funcs_with_args() get_chunks_funcs = [] - for func in all_funcs_kwargs: - if "get_chunks" in all_funcs_kwargs[func]: + for func in all_funcs: + if "get_chunks" in all_funcs[func]: get_chunks_funcs.append(func) return get_chunks_funcs From 64149d01d3c7eb208369e746b46a839697834246 Mon Sep 17 00:00:00 2001 From: Aditi Juneja <91629733+Schefflera-Arboricola@users.noreply.github.com> Date: Sun, 11 May 2025 01:01:41 +0530 Subject: [PATCH 7/7] moved get_all_funcs_with_args in get_functions_with_get_chunks and added test for it. --- nx_parallel/tests/test_get_chunks.py | 66 ++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 19 deletions(-) diff --git a/nx_parallel/tests/test_get_chunks.py b/nx_parallel/tests/test_get_chunks.py index 4fba5a54..8d4010ec 100644 --- a/nx_parallel/tests/test_get_chunks.py +++ b/nx_parallel/tests/test_get_chunks.py @@ -10,28 +10,28 @@ import nx_parallel as nxp -def get_all_funcs_with_args(package_name="nx_parallel"): - """Returns a dict keyed by function names to a list of - the function's args names, for all the functions in - the package `package_name`. - """ - package = importlib.import_module(package_name) - funcs_with_args = {} +def get_functions_with_get_chunks(): + """Returns a list of function names with the `get_chunks` kwarg.""" - for name, obj in inspect.getmembers(package, inspect.isfunction): - if not name.startswith("_"): - signature = inspect.signature(obj) - arguments = [ - param.name - for param in signature.parameters.values() - if param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD - ] - funcs_with_args[name] = arguments - return funcs_with_args + def get_all_funcs_with_args(package_name="nx_parallel"): + """Returns a dict keyed by function names to a list of + the function's args names, for all the functions in + the package `package_name`. + """ + package = importlib.import_module(package_name) + funcs_with_args = {} + for name, obj in inspect.getmembers(package, inspect.isfunction): + if not name.startswith("_"): + signature = inspect.signature(obj) + arguments = [ + param.name + for param in signature.parameters.values() + if param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD + ] + funcs_with_args[name] = arguments + return funcs_with_args -def get_functions_with_get_chunks(): - """Returns a list of function names with the `get_chunks` kwarg.""" all_funcs = get_all_funcs_with_args() get_chunks_funcs = [] for func in all_funcs: @@ -40,6 +40,34 @@ def get_functions_with_get_chunks(): return get_chunks_funcs +def test_get_functions_with_get_chunks(): + # TODO: Instead of `expected` use ALGORTHMS from interface.py + # take care of functions like `connectivity.all_pairs_node_connectivity` + expected = { + "all_pairs_all_shortest_paths", + "all_pairs_bellman_ford_path", + "all_pairs_bellman_ford_path_length", + "all_pairs_dijkstra", + "all_pairs_dijkstra_path", + "all_pairs_dijkstra_path_length", + "all_pairs_node_connectivity", + "all_pairs_shortest_path", + "all_pairs_shortest_path_length", + "approximate_all_pairs_node_connectivity", + "betweenness_centrality", + "closeness_vitality", + "edge_betweenness_centrality", + "is_reachable", + "johnson", + "local_efficiency", + "node_redundancy", + "number_of_isolates", + "square_clustering", + "tournament_is_strongly_connected", + } + assert set(get_functions_with_get_chunks()) == expected + + def test_get_chunks(): def random_chunking(nodes): _nodes = list(nodes).copy()