refactor(SearchSpace): Switch to clone()
for search space.
#94
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR attempts to clean up and document the SearchSpace setup as well as a few other minor optimizations. This is to make progress towards an 1) ask-and-tell interface for benchmarking algorithmic changes/decisions easily and fast, 2) importantly fast, the latest set of changes tried to be self contained to their modules but have sped up things from
6.2seconds
after improvements from for the basic example to1.2seconds
and now to0.7seconds
(excluding import time).All of this is not super important when considering the time to train models but this is unfortunately the difference of being able to quickly ablate and benchmark changes required for both CI and developing new research methods.
The most important contributions of this PR is to avoid the use of
deepcopy
onSearchSpace
objects and instead resort to usingclone()
where possible. This simply just calls the constructor with the arugments and sets the value if any.SearchSpace
as both a space definition and config, then now it's detectable as to where these clones occur.Here the changes this PR makes to a simple sampling in the script below which does a simple generate 1000 configs, 100 times.
I imagine switch to a non-copy and dedicated configuration object could drop this down to
0.05
. For reference, some recent optimization work for ConfigSpace which removed cython based things has it at around0.3
seconds.This is ignoring the fact most optimizers/acq. functions then have to extract all the information and put them in some array structure which is not show in the figure above, and then de-allocate all the clones.
Along with that, there's now documentation and typing so it's been re-enabled in ruff and mypy.
I don't really know what's going on with the graphs under the hood but after some debugging and investigations, these don't share a lot of similarities with a
Parameter
and it's more of a overlap of behaviour then really belonging to the heirarchy.Future PR's from this insight might consider making them seperate classes with no shared implementation such that consuming code can knowingly decide what to do on detection of a
GraphParameter
, e.g. you may not immediatly know you can't mutate some of them or that it can be vectorized to a number/set of numbers, which would be important to know for a PFN-surrogate for example.