Skip to content

Comments

Added grid generation utlity for ConfigSpaces#142

Merged
mfeurer merged 16 commits intoautoml:masterfrom
RaghuSpaceRajan:grid_gen_resolved_conflict
Apr 14, 2021
Merged

Added grid generation utlity for ConfigSpaces#142
mfeurer merged 16 commits intoautoml:masterfrom
RaghuSpaceRajan:grid_gen_resolved_conflict

Conversation

@RaghuSpaceRajan
Copy link
Contributor

No description provided.

@mzeiseSAP
Copy link
Contributor

Two minor remarks:

  • if the defined, the grid generation could utilize the quantization for UniformFloatHyperparameter and UniformIntegerHyperparameter instead of relying on num_steps_dict
  • the function's description could mention that constraints in the config space are ignored when generating the grid

@RaghuSpaceRajan
Copy link
Contributor Author

RaghuSpaceRajan commented Jan 23, 2020

Thanks for the feedback!

  • I did not know about the quantization factor. Would be good to have it as a default step size for the grid. Will take a look at it.
  • I updated the documentation to reflect that conditional HyperParameters are ignored currently. I wanted to add those, but probably in the future at some point.

@Neeratyoy
Copy link

Hi, is this going to be merged?

@mfeurer
Copy link
Contributor

mfeurer commented Mar 9, 2021

Good question @Neeratyoy. We should probably have a look at whether that's possible. @RaghuSpaceRajan is there a reason you chose this to be a class and not a function? Also, to have this merged we'd need some unit tests, especially for testing the behavior when a hyperparameter has a given quantization.

@RaghuSpaceRajan
Copy link
Contributor Author

We should probably have a look at whether that's possible. @RaghuSpaceRajan is there a reason you chose this to be a class and not a function?

On looking at it again, I think it was because there was a function called get_grid_point (in my local workspace) to get a specific point in the generated grid using a list of indices for the different HPs using their "natural" order in ConfigSpace, e.g. passing [0, 2, 1] for a grid of 3 HPs would return the grid point containing the 1st HP's 0th grid value, the 2nd HP's 2nd grid value and the 3rd HP's 1st grid value (0-indexed). I was storing the generated grid as a member variable to not repeat grid creation every time I wanted to query a particular grid point. But then I added grid generation for conditional spaces and "counting" in the generated grid becomes much more involved and I removed get_grid_point.

If you prefer, I can make grid generation be a function again, @mfeurer.

Also, to have this merged we'd need some unit tests, especially for testing the behavior when a hyperparameter has a given quantization.

I will add these soon.

@codecov
Copy link

codecov bot commented Mar 16, 2021

Codecov Report

Merging #142 (6aefd63) into master (5b2392c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #142   +/-   ##
=======================================
  Coverage   68.20%   68.20%           
=======================================
  Files          18       18           
  Lines        1774     1774           
=======================================
  Hits         1210     1210           
  Misses        564      564           

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 5b2392c...6aefd63. Read the comment docs.

int1 = UniformIntegerHyperparameter(name='int1', lower=10, upper=100, log=True)
ord1 = OrdinalHyperparameter(name='ord1', sequence=['1', '2', '3'])

cs.add_hyperparameters([float1, int1, cat1, ord1, const1])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please check for a few extreme cases, too? Such as only numerical, only categorical, only constant, only a single hyperparameter, no hyperparameters yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the case of no hyperparameters, in get_cartesian_product, itertools.product() returns a single empty tuple element (I expected 0 elements) here: https://github.com/RaghuSpaceRajan/ConfigSpace/blob/19144c34b4b0cce2e9df1bb66943bbd17392c818/ConfigSpace/util.pyx#L566 which leads to a single empty Configuration being generated in the grid. Should we check for this and make sure no configuration is returned?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check for this and make sure no configuration is returned?

Yes, I think so.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@RaghuSpaceRajan RaghuSpaceRajan requested a review from mfeurer March 24, 2021 19:34
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.

4 participants