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

Fix trial index returned by get_best_parameters_from_model_predictions_with_trial_index #3284

Closed
wants to merge 1 commit into from

Conversation

saitcakmak
Copy link
Contributor

Summary:
This method has long been returning the index of the GR that was used to extract the best point, rather than returning the index of the trial that generated the best point. I came across it while looking into something else and figured I'd fix it while at it. This method & other existing best point utilities will likely be replaced in the near future, but it doesn't hurt to it work correctly in the mean time.

Fixes #1629

Differential Revision: D68834769

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jan 29, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D68834769

…s_with_trial_index (facebook#3284)

Summary:

This method has long been returning the index of the GR that was used to extract the best point, rather than returning the index of the trial that generated the best point. I came across it while looking into something else and figured I'd fix it while at it. This method & other existing best point utilities will likely be replaced in the near future, but it doesn't hurt to it work correctly in the mean time.

Fixes facebook#1629

Reviewed By: esantorella

Differential Revision: D68834769
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D68834769

saitcakmak added a commit to saitcakmak/Ax that referenced this pull request Jan 29, 2025
…s_with_trial_index (facebook#3284)

Summary:

This method has long been returning the index of the GR that was used to extract the best point, rather than returning the index of the trial that generated the best point. I came across it while looking into something else and figured I'd fix it while at it. This method & other existing best point utilities will likely be replaced in the near future, but it doesn't hurt to it work correctly in the mean time.

Fixes facebook#1629

Reviewed By: esantorella

Differential Revision: D68834769
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 9be04d6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AxClient.get_best_trial() produces the wrong best trial index with use_model_predictions=True.
2 participants