Skip to content

Conversation

jclevesque
Copy link
Contributor

Hyperparameters defined in log space should check for legality in log space as well. If you have a hyperparameter with large possible values, when its scaled value is on the boundaries of the unit space 0-1 it can be determined to have an illegal value when in fact it was legal.

Ex:

scaled_value = 1.000000001 # a reasonable rounding error simulation
upper, lower = 1e10, 1e-10 # large space, help show issue
log_upper, log_lower = np.log(upper), np.log(lower)
hp_value = np.exp(scaled_value * (log_upper - log_lower) + log_lower)
print(hp_value)
> 10000000460.5
print(hp_value > upper)
> True

@mfeurer
Copy link
Contributor

mfeurer commented Nov 2, 2016

Thanks for this PR. Could you please give an example where this happens in practice? I cannot recall observing such a behaviour before. In case the value is sampled, the scaled_value should not exceed 1, right? Currently, such a check is not implemented to avoid transforming from the unit hypercube to the real value for speed reasons.

@jclevesque
Copy link
Contributor Author

Here is a quick test showing what I'm talking about.

import numpy as np

def raw_test_comparison(x, upper=1e5, lower=1e-5):
    log_upper, log_lower = np.log(upper), np.log(lower)
    hp_value = np.exp(x * (log_upper - log_lower) + log_lower)
    print("0-1 hyperparameter value: %f, real hyper value: %f" % (x, hp_value))
    print("Is legal? %s" % (not (hp_value > upper)))

raw_test_comparison(1.)
raw_test_comparison(1., upper=1e10, lower=1e-10)

from ConfigSpace.hyperparameters import UniformFloatHyperparameter

def config_space_comparison(x, upper=1e5, lower=1e-5):
    hyper = UniformFloatHyperparameter('test', lower=lower, upper=upper, log=True)
    hp_value = hyper._transform(x)
    print("0-1 hyperparameter value: %f, real hyper value: %f" % (x, hp_value))
    print("Using ConfigSpace, is legal?:", hyper.is_legal(hp_value))

config_space_comparison(1.)
config_space_comparison(1., upper=1e10, lower=1e-10)

Outputs on my machine (archlinux, python 3.5.2, numpy 1.11.2):

0-1 hyperparameter value: 1.000000, real hyper value: 100000.000000
Is legal? False
0-1 hyperparameter value: 1.000000, real hyper value: 10000000000.000004
Is legal? False
0-1 hyperparameter value: 1.000000, real hyper value: 100000.000000
Using ConfigSpace, is legal?: False
0-1 hyperparameter value: 1.000000, real hyper value: 10000000000.000004
Using ConfigSpace, is legal?: False

@mfeurer
Copy link
Contributor

mfeurer commented Nov 3, 2016

Okay, now I get it. Thanks for reporting this. I need to check whether this would drastically reduce sampling speed of the ConfigSpace.

@mfeurer
Copy link
Contributor

mfeurer commented Nov 3, 2016

On some simple tests, this doesn't seem to decrease the performance drastically. Would you mind adding some unit tests before I can merge it?

@jclevesque
Copy link
Contributor Author

I added a single unit test to test_hyperparameters.py with the same examples shown above. Let me know if this works for you!

@mfeurer
Copy link
Contributor

mfeurer commented Nov 18, 2016

Thanks for the test. In your examples, you also showed counter examples. Is it possible to add them to the unittests as well?

@jclevesque
Copy link
Contributor Author

jclevesque commented Dec 19, 2016

Hey, sorry about the delay. I had other obligations, then forgot about it. What do you mean by counter examples?

Apart from testing the boundaries of a space (that is, 0 and 1), I don't see the benefit of adding other tests. Could add 0.5, but if 0 and 1 are legal, 0.5 should always be legal as well.

Potential unit test:

def test_log_space_conversion(self):
    lower, upper = 1e-5, 1e5
    hyper = UniformFloatHyperparameter('test', lower=lower, upper=upper, 
        log=True)
    self.assertTrue(hyper.is_legal(hyper._transform(0.)))
    self.assertTrue(hyper.is_legal(hyper._transform(0.5)))
    self.assertTrue(hyper.is_legal(hyper._transform(1.)))

    lower, upper = 1e-10, 1e10
    hyper = UniformFloatHyperparameter('test', lower=lower, upper=upper, 
        log=True)
    self.assertTrue(hyper.is_legal(hyper._transform(0.)))
    self.assertTrue(hyper.is_legal(hyper._transform(0.5)))
    self.assertTrue(hyper.is_legal(hyper._transform(1.)))

@mfeurer
Copy link
Contributor

mfeurer commented Jan 11, 2017

Hm, good point. I'll go ahead and merge this.

@coveralls
Copy link

coveralls commented Jan 11, 2017

Coverage Status

Coverage increased (+0.04%) to 67.058% when pulling d225654 on jclevesque:master into f03b70f on automl:master.

@mfeurer mfeurer merged commit 854f70b into automl:master Jan 11, 2017
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

Successfully merging this pull request may close these issues.

3 participants