Skip to content

Allow using forbidden clauses with non-numeric values#230

Merged
mfeurer merged 2 commits intoautoml:masterfrom
filipbartek:fix/forbidden-str
Feb 8, 2022
Merged

Allow using forbidden clauses with non-numeric values#230
mfeurer merged 2 commits intoautoml:masterfrom
filipbartek:fix/forbidden-str

Conversation

@filipbartek
Copy link
Copy Markdown
Contributor

@filipbartek filipbartek commented Jan 31, 2022

Before

Restricting a categorical hyperparameter with non-numeric (namely string) values by a forbidden clause forbidden and calling forbidden.is_forbidden on a relevant instantiated_hyperparameters dictionary raises the following exception:

TypeError: must be real number, not str

After

Forbidden clauses that involve non-numeric hyperparameters can be evaluated using is_forbidden without raising an exception.

Both of these tests currently raise an exception:
> TypeError: must be real number, not str
This commit solves the two failing tests.
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 31, 2022

Codecov Report

Merging #230 (b1390d0) into master (5842d4d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #230   +/-   ##
=======================================
  Coverage   67.11%   67.11%           
=======================================
  Files          17       17           
  Lines        1627     1627           
=======================================
  Hits         1092     1092           
  Misses        535      535           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5842d4d...b1390d0. Read the comment docs.

@mfeurer
Copy link
Copy Markdown
Contributor

mfeurer commented Feb 7, 2022

Thanks for catching this. I can reproduce the issue and am currently checking your proposed fix. It looks good.

I have one question, though: it appears that Condition.is_forbidden is only invoked by unit tests. How did you find this issue?

PS: Your other PR is next on my list.

@filipbartek
Copy link
Copy Markdown
Contributor Author

I implemented a procedure that samples configuration according to a dynamic probability distribution. This procedure calls is_forbidden to prune the configuration space.

@mfeurer
Copy link
Copy Markdown
Contributor

mfeurer commented Feb 8, 2022

Got it, thanks for the explanation. I could find no performance decrease when benchmarking the sampling speed, so let's merge this :)

@mfeurer mfeurer merged commit 5e1d3a5 into automl:master Feb 8, 2022
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.

2 participants