-
Notifications
You must be signed in to change notification settings - Fork 120
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
TarjanTest.test_4 failed #408
Comments
This could be a bug, but we should try to execute it on other platform or with other compiler. |
Another help could be write tests for some other cases where this bug exist. |
Sure, I shall try to fix this bug, since the reason why this happens is possibly figured out by the debug information. |
Considering different platforms or compilers, the order of cachedAdjMatrix and other data stored and visited may differ, I conjecture, which I have no evidence, due to constraint on machines at hand. A direct suggestion would be to provide a random implementation on the order of graph data stored or visited, to generate a variety of unit test cases, which are equivalent to the cases already exists, since our algorithms are independent of the order of data. And of course new tests are still needed. |
The following is to explain in detail why the current master(commit 994e9b9) failed TarjanTest.test_4 on my environment. For convenience we copy the case here. According to the debug information, the list of (node, its discoveryTime) is: When the current node is 10 and the neighbor node is 18, |
Describe the bug
Unit Test 1 FAILED TEST
[ RUN ] TarjanTest.test_4
/mnt/CXXGraph/test/TarjanTest.cpp:185: Failure
Expected equality of these values:
res.verticeBiconnectedComps.size()
Which is: 6
expectRes.size()
Which is: 7
[ FAILED ] TarjanTest.test_4 (0 ms)
To Reproduce
in my environment(CentOS in Docker on macOS), I modified
zlib version from 1.2.13 to 1.2.12 for cmake
in order to pass cmake&&make.
I tried TarjanTest.test_4 on master commit 994e9b9, v3.1.0, v3.0.0,
which all have the same problem.
Expected behavior
expectRes.size()
Which is: 7
Screenshots
Desktop (please complete the following information):
Additional context
I'm not sure whether it's a bug or just something to do with my environment.
The following is my debug information:
For the case TarjanTest.test_4
The result I got misses the vbcc [10,17,18].
The problem seems to be caused by the lowestDisc of node 10 modified earlier than the time when it finishes its dfs_helper.
The text was updated successfully, but these errors were encountered: