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

Move make_benchmark_graphs.py scripts from atomspace to benchmark repo #7

Closed
vsbogd opened this issue Apr 27, 2018 · 6 comments
Closed

Comments

@vsbogd
Copy link
Contributor

vsbogd commented Apr 27, 2018

File make_benchmark_graphs.py is part of opencog/benchmark/atomspace/atomspace benchmarkin.
Todo:

  • move make_benchmark_graphs.py into benchmark repo
  • fix atomspace/atomspace/README.md using new file location
@linas
Copy link
Member

linas commented Apr 27, 2018

Yeah, move the contents of https://github.com/opencog/atomspace/blob/master/scripts/query as well. Honestly, I didn't even know that these scripts existed, until just now. The vast, incredible power of cruft-accumulation.

@vsbogd
Copy link
Contributor Author

vsbogd commented Apr 29, 2018

Only two of them are related to benchmarking:

  • atomspace/scripts/query/benchmark_query.sh
  • atomspace/scripts/benchmark_utests.sh
    Both scripts run unit tests in a loop and show timings. I would replace them by benchmarks. @ngeiswei, it seems you are an author, what do you think?

@ngeiswei
Copy link
Member

ngeiswei commented Apr 30, 2018

I see two problems with moving them to the benchmark repo

  1. it's harder to find the build directory of the atomspace repo from the benchmark repo,
  2. unit tests performances are not intended to reflect real world performances.

So if they are being moved, to minimize these problems I suppose one could

  1. assume by default that the atomspace repo folder is beside the benchmark repo folder, that will probably work for most people,
  2. printing a big warning of its shortcomings in the corresponding README.md and/or as a message at script launch.

If that is respected I think it would be acceptable to move them to the benchmark repo.

@vsbogd
Copy link
Contributor Author

vsbogd commented May 3, 2018

@ngeiswei, I didn't mean these scripts should be literally "moved". I think more proper way is adding new benchmarks (for instance under #8) and removing these shell scripts.

@linas
Copy link
Member

linas commented May 25, 2018

this has been done, so this bug can be closed, right?

@vsbogd
Copy link
Contributor Author

vsbogd commented May 28, 2018

Yes, closing it, new tests to be added under #8

@vsbogd vsbogd closed this as completed May 28, 2018
ngeiswei added a commit to ngeiswei/benchmark that referenced this issue Jul 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants