Validate the configuration produced by LlamaTune inverse_transform() #806
Conversation
motus
added
bug
There was a problem hiding this comment.
Somewhat unrelated, we should add "quantization + llamatune" tests as well.
| @@ -139,6 +139,8 @@ def inverse_transform(self, configurations: pd.DataFrame) -> pd.DataFrame: | |||
| self.target_parameter_space, | |||
| vector=target_config_vector, | |||
| ) | |||
| self.target_parameter_space.check_configuration(target_config) | |||
There was a problem hiding this comment.
Might just need to clip them to the range in the case that it's too large (e.g., due to quantization issues).
Are there other failure cases than that?
There was a problem hiding this comment.
I took a slightly closer look at this.
max_unique_values_per_param is forcing the lower dimensions space to be UniformIntegerHyperparatmers, but the vector produced that is then dot producted with the approximate inverted matrix returns floats.
So, I think there's an additional scaling operation required to put that vector back into the max_unique_values_per_param range when that option is not None.
There was a problem hiding this comment.
Ok, I've added the q_scaler inverse transform here as well, which makes the values look much better, but one is still out of bounds. I think it's maybe just needing clipping either there, or in a previous step.
There was a problem hiding this comment.
pytest -n0 mlos_bench/mlos_bench/tests/optimizers/llamatune_opt_test.pyto reproduce
These changes are mainly to add flexibility to the auth service in supporting azure authenticating via: - A personal identity via az CLI for the local dev environment. This will still run into expiring issues, but is probably fine for local dev. - A personal or managed identity in an azure hosted environment. The flexibility to use the managed identity allows us to avoid the expiring issue without needing a separate SP, key-vault and cert setup, aligning with new security recommendations. --------- Co-authored-by: Eu Jing Chua <eujingchua@microsoft.com> Co-authored-by: Sergiy Matusevych <sergiym@microsoft.com>
Mypy got updated to v 1.11 recently We need these mypy fixes to make other PRs pass the CI tests
Closes microsoft#785 Also, add more unit tests to make sure the optimizer handles string inputs correctly. --------- Co-authored-by: Brian Kroth <bpkroth@users.noreply.github.com>
| @@ -261,7 +313,7 @@ def _transform(self, configuration: dict) -> dict: | |||
| # Clip value to force it to fall in [0, 1] | |||
| # NOTE: HeSBO projection ensures that theoretically but due to | |||
| # floating point ops nuances this is not always guaranteed | |||
| value = max(0.0, min(1.0, norm_value)) # pylint: disable=redefined-loop-name | |||
| value = max(0.0, min(1.0, norm_value)) | |||
There was a problem hiding this comment.
unnecessary pylint exception?
|
Taking this over as a fix as well in a new PR - #812 |
…812) Revamp of #806 with the fix as well. Adds code to handle appropriate scaling and type conversion and checking of the inverted config. Splits that code out to a separate method for individual configs to make pylint happy and improve code readability. --------- Co-authored-by: Sergiy Matusevych <sergiym@microsoft.com> Co-authored-by: Eu Jing Chua <chuaeujing@gmail.com> Co-authored-by: Eu Jing Chua <eujingchua@microsoft.com> Co-authored-by: Sergiy Matusevych <sergiy.matusevych@gmail.com>
Code to reproduce #805