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

Getting a random value from a Domain #2246

Closed
Neabfi opened this issue May 19, 2019 · 2 comments · Fixed by #2259
Closed

Getting a random value from a Domain #2246

Neabfi opened this issue May 19, 2019 · 2 comments · Fixed by #2259

Comments

@Neabfi
Copy link

Neabfi commented May 19, 2019

It would be convenient to have a single method to get a random value from a Domain. It would make the code to do a random search shorter and prettier.

Would that make sense? If yes, I can create a PR.
(cc @wchargin )

Examples:

class RealInterval(Domain):
  ...
  def rand(self):
    return random.randint(self.minvalue, self.maxvalue)
  ... 
class Discrete(Domain):
  ...
  def rand(self):
    return random.choice(self.values)
  ... 
@wchargin
Copy link
Contributor

Hi @Neabfi! Thanks for the feedback. I considered this, and a previous
draft of the APIs included Domain.sample_uniform
. I opted to cut
it for a few reasons, which all boil down to the following: while we
provide a vocabulary for describing domains, we don’t provide any actual
tuning algorithms (nor do we currently plan to), and I didn’t want to
give an impression to the contrary. That is, one might expect that if we
provide random sampling, which is basically a random-search tuner in all
but name, we must surely provide something more sophisticated (à la
Vizier), but this is not the case.

Note that you can write your own sample_uniform, as in the demo
(which is separate from the tutorial notebook), which streamlines the
rest of your model code. But I’m sympathetic to the point of view that
even this is boilerplate that the library should provide. I’ll bring
this up tomorrow and get some other opinions internally.

wchargin added a commit that referenced this issue May 21, 2019
Summary:
Resolves #2246.

Test Plan:
Unit tests added, and demo updated; the demo still works (after
cherry-picking #2258).

wchargin-branch: hparams-sample-uniform
@wchargin
Copy link
Contributor

I’ll bring this up tomorrow and get some other opinions internally.

This is fine with us. Sent #2259.

If yes, I can create a PR.

Thank you for the offer—we appreciate it! In this case there were some
particular tests that I wanted to include, so I went ahead and created
the PR myself, but in general we’d be more than happy to accept your
contributions!

wchargin added a commit that referenced this issue Jun 4, 2019
Summary:
Resolves #2246.

Test Plan:
Unit tests added, and demo updated; the demo still works.

wchargin-branch: hparams-sample-uniform
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants