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

Support for special tunable values outside of the range #617

Merged
merged 53 commits into from
Jan 17, 2024

Conversation

motus
Copy link
Member

@motus motus commented Dec 1, 2023

Enables special values outside of the range (e.g., -1 with a range of [0, 100]).

To do we make use of "conditionals" in ConfigSpace to constrain the space. This has a number of implementation implications, addressed below:

  • Add support for special values to the Tunable class
  • Add unit tests for assigning special values outside of the range to the Tunable objects
  • Add special values outside of the range to the unit tests for ConfigSpace conversion
  • Implement proper TunableGroups to ConfigSpace conversion for tunables with special values
  • Update mlos_core optimizers to support conditionals and special values in ConfigSpace
  • Add more unit tests to check the conversion
  • Make LlamaTune adapter support conditionals in ConfigSpace

@motus motus added the WIP Work in progress - do not merge yet label Dec 1, 2023
motus added 24 commits November 30, 2023 17:22
…values; implement TunableGroups to ConfigSpace conversion.

TODO: convert from ConfigSpace back to Tunablegroups
@motus motus removed the WIP Work in progress - do not merge yet label Jan 10, 2024
Copy link
Contributor

@bpkroth bpkroth left a comment

Choose a reason for hiding this comment

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

A few nits here and there, but I think the main thing we should revisit is whether to support mixed numerical and string types for special values.

Copy link
Contributor

@bpkroth bpkroth left a comment

Choose a reason for hiding this comment

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

LGTM

I left one place to add a comment that probably needs some pylint checks for spacing.

I think one tweak we/I could add in a future PR would be to update the json schemas to account for disallowing ! characters in the names, but that doesn't have to be here.

@motus motus merged commit 5daaa26 into microsoft:main Jan 17, 2024
11 checks passed
@motus motus deleted the sergiym/tunable/special branch January 17, 2024 21:29
bpkroth added a commit that referenced this pull request Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants