networkx / nx-parallel

A networkx backend that uses joblib to run graph algorithms in parallel.
BSD 3-Clause "New" or "Revised" License
23 stars 20 forks source link

unexpected failing of `test_edge_current_flow_betweenness_partition` test with windows-latest in nx-parallel #36

Closed Schefflera-Arboricola closed 7 months ago

Schefflera-Arboricola commented 7 months ago

The test_edge_current_flow_betweenness_partition test is failing with windows-latest in nx-parallel repo, but this is not happening in the main networkx repo. Also, nothing seems wrong with the test. It has been failing for the commits i pushed in different PRs in the last few hours.

PR #35 , #32 , #27

=========================== short test summary info ===========================
FAILED algorithms/community/tests/test_divisive.py::test_edge_current_flow_betweenness_partition - assert {0, 1, 2, 3} in [{3, 4, 5, 6}, {0, 1, 2}]
===== 1 failed, 5260 passed, 57 skipped, 11 warnings in 115.37s (0:01:55) =====
Error: Process completed with exit code 1.
dschult commented 7 months ago

It looks like there are two valid solutions to the problem presented in the test. The barbell graph is symmetric. So one solution puts the middle node with the left and the other puts it with the nodes on the right. I haven’t checked this for sure, but it makes sense based on the symmetry of the problem. We don’t see this error in NetworkX because the implementation only finds one of the solutions every time. I’m not sure why the parallel implementation would find a different solution on windows than on the *nix based platforms.

I guess the correct thing to is: 1) verify that there are two solutions. 2) rewrite the test so that it passes with either solution.

answer = [[{0, 1, 2, 3}, {4, 5, 6}], [{0, 1, 2}, {3, 4, 5, 6}]]
assert len(C) == len(answers[0])
assert any(all(s in answer for s in C) for answer in answers)
Schefflera-Arboricola commented 7 months ago

I’m not sure why the parallel implementation would find a different solution on windows than on the *nix based platforms.

It might have something to do with the fact that networkx tests run on "windows" and in nx-parallel we have "windows-latest". So, these 2 OSs might be running the function in different ways.

I tried changing it to "windows" in nx-parallel(https://github.com/Schefflera-Arboricola/nx-parallel/pull/1 and https://github.com/networkx/nx-parallel/pull/39) but I think it has to be configured at the repository owner side in someway. I don't know much about it, i'm just guessing.

MridulS commented 7 months ago

Yes, it does seems to be the OS issue, in https://github.com/networkx/nx-parallel/actions/runs/7656063469/job/20863515922 it passes on windows.

dschult commented 7 months ago

If there is a different result on windows and windows-latest (and the produced result is actually correct, but not the solution that the test was looking for, then we should probably make the tests check for either of the valid solutions. networkx/networkx#7263 attempts to do this. I would prefer this solution to just changing the OS we are testing under (when we know the other one doesn't pass the tests).

Can you take a look at networkx/networkx#7263? If we merge that then I think these tests will start passing.