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

Handle invalid neighbors in local search gracefully #773

Merged
merged 4 commits into from
Sep 23, 2021

Conversation

filipbartek
Copy link
Contributor

@filipbartek filipbartek commented Sep 13, 2021

The iterator returned by ConfigSpace.util.get_one_exchange_neighborhood may visit an invalid configuration. If it does so, it either raises a ValueError (with the probability 0.05) or yields the configuration (otherwise). This pull request ensures that LocalSearch._do_search handles these situations gracefully.

  1. If ValueError is raised, it is caught and ignored (instead of crashing smac).
  2. If an invalid configuration is visited and scores better than the current candidate, it is ignored (instead of replacing the current candidate).

The script run.sh in demo.zip demonstrates raising of ValueError from the iterator. The error is raised with a probability greater than 0 and smaller than 1.

Issue at ConfigSpace that discusses the non-determinism of the configuration validation: automl/ConfigSpace#194

If the iterator returned by
`ConfigSpace.util.get_one_exchange_neighborhood`
reaches an invalid configuration,
it raises a `ValueError` with the probability 5 %.
The randomness is sampled using `np.random.random()`,
so it is non-deterministic.
See ConfigSpace/util.pyx:218.

This commit ensures that the exception is caught and handled gracefully.
In local search, only advance to a better-scoring neighbor
if that neighbor is a valid configuration.

Note that `neighborhood_iterator` may yield an invalid configuration.
@codecov
Copy link

codecov bot commented Sep 13, 2021

Codecov Report

Merging #773 (99d19c8) into development (c4a745e) will decrease coverage by 0.05%.
The diff coverage is 80.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #773      +/-   ##
===============================================
- Coverage        87.02%   86.97%   -0.06%     
===============================================
  Files               68       68              
  Lines             6221     6233      +12     
===============================================
+ Hits              5414     5421       +7     
- Misses             807      812       +5     
Impacted Files Coverage Δ
smac/optimizer/ei_optimization.py 90.00% <78.94%> (-1.21%) ⬇️
smac/configspace/__init__.py 100.00% <100.00%> (ø)
smac/intensification/intensification.py 95.43% <0.00%> (-0.37%) ⬇️

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 c4a745e...99d19c8. Read the comment docs.

@dengdifan
Copy link
Contributor

Looks good to me! Thanks for the contribution!

@mfeurer
Copy link
Contributor

mfeurer commented Sep 16, 2021

@dengdifan just wanted to make you aware that this only fixes symptoms that are caused by a bug in the ConfigSpace and should be fixed there. Therefore, once they are fixed, this change should be rolled back (and you should already create an issue for that ore something else to remember to undo these changes).

@dengdifan dengdifan merged commit 45daebb into automl:development Sep 23, 2021
@filipbartek filipbartek deleted the issue/invalid-neighbor branch September 23, 2021 14:25
renesass pushed a commit that referenced this pull request Oct 20, 2021
* Documentation was updated thoroughly. A new theme with a new structure is provided and all pages have been updated. Also, the examples revised and up-to-date.
* Option to use an own stopping strategy using `IncorporateRunResultCallback`.
* Changed `scripts/smac` to `scripts/smac.py`.
* `README.md` updated.
* `CITATION.cff` added.
* Made `smac-validate.py` consistent with runhistory and tae. (#762)
* `minR`, `maxR` and `use_ta_time` can now be initialized by the scenario. (#775)
* `ConfigSpace.util.get_one_exchange_neighborhood`'s invalid configurations are ignored. (#773)
* Fixed an incorrect adaptive capping behaviour. (#749)
* Avoid the potential `ValueError` raised by `LocalSearch._do_search`. (#773)

Co-authored-by: dengdifan <[email protected]>
Co-authored-by: Filip Bártek <[email protected]>
Co-authored-by: Thomas Holvoet <[email protected]>
Co-authored-by: benjamc <[email protected]>
Co-authored-by: Carolin Benjamins <[email protected]>
Co-authored-by: Marius Lindauer <[email protected]>
Co-authored-by: eddiebergman <[email protected]>
Co-authored-by: Difan Deng <[email protected]>
Co-authored-by: dengdifan <[email protected]>
Co-authored-by: Tim Ruhkopf <[email protected]>
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