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

Refactor geometric_soft_configuration_model tests for performance #7210

Merged
merged 2 commits into from Jan 10, 2024

Conversation

rossbar
Copy link
Contributor

@rossbar rossbar commented Jan 5, 2024

#6858 added the geometric_soft_configuration_model generator, which included several tests that generated graphs with >1k nodes. These tests were quite slow:

$ pytest --durations 4 networkx/generators/tests/test_geometric.py

...

4.07s call     networkx/generators/tests/test_geometric.py::test_mean_degree_S1
4.06s call     networkx/generators/tests/test_geometric.py::test_mean_kappas_S1
1.34s call     networkx/generators/tests/test_geometric.py::test_compare_mean_kappas_different_gammas_S1
0.18s call     networkx/generators/tests/test_geometric.py::test_dict_kappas_S1

These could be decorated with pytest.mark.slow, but then they'd still be running in the coverage job in CI. Since the tests are pseudo-stochastic anyways (i.e. they test average properties of graphs created with a seed) I figured there was no harm in refactoring them instead to improve performance. The main thing was to drop the total number of nodes from 1000's to 10's (reducing the mean_degree as well, where necessary). I also consolidated two of the tests into one so that the input graph is only created once. These changes result in:

$ pytest --durations 4 networkx/generators/tests/test_geometric.py

...

0.18s call     networkx/generators/tests/test_geometric.py::test_dict_kappas_S1
0.03s call     networkx/generators/tests/test_geometric.py::TestRandomGeometricGraph::test_number_of_nodes
0.03s call     networkx/generators/tests/test_geometric.py::TestNavigableSmallWorldGraph::test_navigable_small_world
0.01s call     networkx/generators/tests/test_geometric.py::test_beta_clustering_S1

... removing the bottlenecks and dropping the total test runtime for the module from ~10s -> 0.4s (on my machine).

@rossbar
Copy link
Contributor Author

rossbar commented Jan 5, 2024

In CI the savings are even more pronounced: the total test runtime on ubuntu is about 30 sec shorter (25%) for this branch compared to main.

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.

Excellent! Nice work!
Thanks for working through this to find smaller numbers that provide similar tests.
Ready to merge from my perspective.

@MridulS MridulS added this to the 3.3 milestone Jan 10, 2024
@MridulS MridulS merged commit 205cd05 into networkx:main Jan 10, 2024
40 checks passed
cvanelteren pushed a commit to cvanelteren/networkx that referenced this pull request Apr 22, 2024
…tworkx#7210)

* TST: Consolidate mean kappa/deg test and use smaller graph.

* TST: Reduce size of test graphs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants