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

FIX: Fixed some issues around vb_suite #9332

Merged
merged 2 commits into from
Jan 22, 2015

Conversation

ledmonster
Copy link

I'm new to vb_suite, and ran following command according to this document.

./test_perf.sh -b master -t HEAD

But I got some errors (InvalidGitRepositoryError and Benchmark duplication errors), and finally I got tests run by fixing following issues.

  • VB_DIR pointed wrong directory
  • temporary variable bmark in groupby was collected as Benchmark
  • indexing module had same name ('series_getitem_scalar') Benchmarks

I'm not so familier with vb_suite, and any feedbacks are welcome ! Thanks.

* temporary variable ``bmark`` in groupby was collected as Benchmark
* indexing module had same name ('series_getitem_scalar') Benchmarks
* VB_DIR pointed wrong directory
@jreback
Copy link
Contributor

jreback commented Jan 22, 2015

was it hanging on something particular?

fyi, you can also run with -r group for example to only run some of the benches

@jreback jreback added the Performance Memory or execution speed performance label Jan 22, 2015
@jreback jreback added this to the 0.16.0 milestone Jan 22, 2015
@@ -559,3 +559,6 @@ def inject_bmark_into_globals(bmark):
for func_name in no_arg_func_list:
bmark = make_large_ngroups_bmark(ngroups, func_name)
inject_bmark_into_globals(bmark)

# avoid bmark to be collected as Benchmark object
del bmark
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without this fix, I got following error:

$ rm vb_suite/benchmarks.db
$ ./test_perf.sh -b master -t HEAD -r groupby
...
2015-01-22 20:47:50,103 [INFO  ] Writing new benchmark groupby_ngroups_100_value_counts, b88a3ca315564ed6afdd935e4d2dc8ad (runner.py:153)
2015-01-22 20:47:50,105 [INFO  ] Writing new benchmark groupby_ngroups_10000_value_counts, f7a6e1f815904619a0428842d3fa2ffe (runner.py:153)
2015-01-22 20:47:50,107 [INFO  ] Writing new benchmark groupby_ngroups_10000_sum, 973f817c1bd057ddc5476e5af7667ed6 (runner.py:153)
2015-01-22 20:47:50,118 [INFO  ] Writing new benchmark groupby_ngroups_10000_value_counts, f7a6e1f815904619a0428842d3fa2ffe (runner.py:153)
...
sqlalchemy.exc.IntegrityError: (IntegrityError) UNIQUE constraint failed: benchmarks.checksum u'INSERT INTO benchmarks (checksum, name, description) VALUES (?, ?, ?)' ('f7a6e1f815904619a0428842d3fa2ffe', 'groupby_ngroups_10000_value_counts', None)

This is because suite module collects all Benchmark objects in groupby module, while temporary variable bmark presents outside the for loop.

@ledmonster
Copy link
Author

Sorry for mashing up multiple modifications in one pull request. Problems I got are like commented above. As to indexing module, I got unexpected result now, and so I'll investigate on it and comment about it later. It seems that, if you already have a correct database, you doesn't get UNIQUE Constraint issue above.

fyi, you can also run with -r group for example to only run some of the benches

thanks.

bm_getitem = Benchmark(statement, setup, ncalls=100000,
name='series_getitem_scalar')
name='time_series_getitem_scalar')
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was same Benchmark name as L220 one. This doesn't cause error, but confusing.

@ledmonster
Copy link
Author

As to indexing module, I got unexpected result now, and so I'll investigate on it and comment about it later.

Investigated, and added commit for that.

jreback added a commit that referenced this pull request Jan 22, 2015
FIX: Fixed some issues around vb_suite
@jreback jreback merged commit 1748bff into pandas-dev:master Jan 22, 2015
@jreback
Copy link
Contributor

jreback commented Jan 22, 2015

@ledmonster thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Performance Memory or execution speed performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants