Thanks a lot for the patch. Unfortunately, I cannot confirm any speed improvements on my machine. I tried the unit tests in test.test_converters_and_test_searchspaces which are somewhat realistic and also the script scripts.benchmark_sampling and found that the tests go from 41s to 42s, while the benchmark sampling stays approximately the same. Could you please check these numbers, too, and also paste the output of the script benchmark_sampling.py with and without the change?

My outputs with this PR:

###
/home/feurerm/sync_dir/projects/ConfigSpace/test/test_searchspaces/auto-sklearn_2017_11_17.pcs
Average time sampling 100 configurations 0.01909217834472656
Average time retrieving a nearest neighbor 0.005693285465240478
Average time checking one configuration 0.00031762908805500376

without:

###
/home/feurerm/sync_dir/projects/ConfigSpace/test/test_searchspaces/auto-sklearn_2017_11_17.pcs
Average time sampling 100 configurations 0.018399810791015624
Average time retrieving a nearest neighbor 0.0057344818115234375
Average time checking one configuration 0.00029520517346834894

I have some further ideas on how to improve the performance of the transform functions that I'd be happy to share if you're interested, but they might take a bit more time to implement.

Thanks, I will look into it soon! And I would be happy to hear your ideas.

I think a lot of slowdown here comes from having a single method to handle both the array and the non-array case, which results in quite some overhead.

Instead, if one would have a function _transform_vector and _transform_scalar one could save quite some time because one could also make the functions cpdef functions as there would only be a single possible output type. An alternative implementation could also use only arrays to not have two strains of logic.

Hi @mfeurer, I investigated the benchmark_sampling.py results. It seems that the times are indeed the same. There are two explanations:

• The time it costs to execute the transform function is insignificant compared to the other code in sample_configuration(). Executing the transform function takes a factor of 10^-6 to 10^-7 seconds, while the average time of sampling 100 configurations takes 0.018 seconds, or 0.00018 ≈ 10^-4 seconds per configuration.
• 723d3cd already includes a performance improvement for Uniform Float Hyperparameters. Furthermore, the Uniform Integer Hyperparameters were already optimized and the Categorical Hyperparameters stayed the same.

However, looking at the _transform() function independently, I did see an improvement in speed. For my tests, I used the _transform() function of the Uniform Float Hyperparameter. With 723d3cd, we get a speedup of a factor ~2.7. With the current pull request, we get a speedup of ~8.7 and with your idea we get a speed up of ~18.2 😃

I could split all the _transform() functions into _transform_vector() and _transform_scalar() and update the PR. And I could also try to figure out when and why the performance of the transform function affects the overall performance, as it does for example seem to speedup running all test-cases. What do you think?

With the current pull request, we get a speedup of ~8.7 and with your idea we get a speed up of ~18.2

That sounds great!

Executing the transform function takes a factor of 10^-6 to 10^-7 seconds, while the average time of sampling 100 configurations takes 0.018 seconds, or 0.00018 ≈ 10^-4 seconds per configuration.

Keep in mind that this is also important for generating nearest neighbor (for SMAC), and I assume it has a larger effect there.

And I could also try to figure out when and why the performance of the transform function affects the overall performance, as it does for example seem to speedup running all test-cases.

I think this would be most important.

I could split all the _transform() functions into _transform_vector() and _transform_scalar() and update the PR.

From a maintenance point I would definitely be in favor of this refactoring as it will make the code clearer an replace a few if/else statements. One can then also start typing the transform functions.

So I'd definitely appreciate an updated PR, but I think you need to decide if it's worth for you splitting up the other transform functions.

Hi @mfeurer, I have split everything into scalar and vector transforms and debugged most problems I encountered, but I'm still failing one check: in test_sample_UniformFloatHyperparameter it seems that not all bins are filled.

For example, the second counts_per_bin list looks like this: [9245, 0, 9025, 0, 9300, 0, 8997, 0, 8991, 0, 9001, 0, 9113, 9056, 0, 0, 9118, 9052, 0, 0, 9102].

Do you have an idea of what could be wrong here?

Do the unit tests work locally for you? Currently, they are killed on travis-ci which makes it really hard to see what's going on. Could you please check?

Hi @mfeurer, it turned out to be a problem with precision. Cython uses 32 bit floats & Python uses 64 bit floats, and this caused the Cython floats to be rounded down to the wrong bucket. So the solution was to use double's instead of float's. And I fixed some other problems as well.

Ah, I just realized what "requested changes" actually means. Didn't see the changed code before. Are your changes also included now?

Ah, I just realized what "requested changes" actually means. Didn't see the changed code before. Are your changes also included now?

Sorry, I don't get this. Did I change some part of the code?

Oh nevermind, I thought you did. I got confused because there was a button that said "View Changes".