Skip to content

[BUG] Benchmarks run same test 12 times #28

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

Closed
magnushiie opened this issue Sep 15, 2022 · 2 comments
Closed

[BUG] Benchmarks run same test 12 times #28

magnushiie opened this issue Sep 15, 2022 · 2 comments
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@magnushiie
Copy link
Contributor

Describe the bug
The benchmarks (like BenchmarkShortestPath) execute 12 iterations with k = 0..11, calculate an n = 2^k, but then don't use either of them in the benchmark except for the title.

To Reproduce
Run the benchmarks and read source code.

Expected behavior
Either n or k should be used in the benchmark core (not sure what these could be for BenchmarkShortestPath, for BenchmarkShortestPathOneToMany it could be the number of target nodes).

@magnushiie magnushiie added bug Something isn't working help wanted Extra attention is needed labels Sep 15, 2022
@LdDl
Copy link
Owner

LdDl commented Sep 15, 2022

Wow, we did miss that. Thank you
Started #29

  1. For One-To-One bench we suggest generate graph vertices based on n=2^k - You can take a look already.

Other works in that PR would be:
2. For One-To-Many it depends. May be 2 benchmarks: one is for different graph sizes; second - number of target nodes
3. We need to re-evaluate BenchmarkShortestPathOneToMany and BenchmarkOldWayShortestPathOneToMany (we need to take a look at #27 also) since we've changed PC specs of developer machine

@LdDl
Copy link
Owner

LdDl commented Sep 15, 2022

Well, we've just updated PR.
Summary:

  1. BenchmarkShortestPath - generates graph with n=2^k, computes its contraction hierarchies, takes two random vertices and computes shortest path between them.
  2. BenchmarkStaticCaseShortestPath - just single run on predefined graph (187k vertices) with computing of contraction hierarchies
  3. BenchmarkShortestPathOneToMany (and old derivative ...OldWay...) - generates graph with n=2^k, computes its contraction hierarchies, takes one random vertex as source and five random vertices as targets and computes shortest paths
  4. BenchmarkTargetNodesShortestPathOneToMany (and old derivative ...OldWay...) - takes predefined graph (187k vertices), computes its contraction hierarchies, takes predefined vertex as source, takes random number (1 up to 2^k) of random vertices, computes shortest paths
  5. BenchmarkStaticCaseShortestPathOneToMany (and old derivative ...OldWay...) - just single run (1 predefined source - 5 predefined targets) on predefined graph (187k vertices) with computing of contraction hierarchies

I believe bug has been fixed, but for sure now we can improve tests (e.g. better random graph generation)

LdDl added a commit that referenced this issue Sep 15, 2022
@LdDl LdDl closed this as completed Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants