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

Add init_position argument to UniformGenerator #2686

Closed
wants to merge 2 commits into from

Conversation

saitcakmak
Copy link
Contributor

Summary:
SobolGenerator uses init_position to ensure that when the model is reconstructed, it resumes candidate generation from where it was left (rather than starting from the beginning of the sequence). Without this, when deduplicate=False, the model would generate the same points it has already generated, which would lead to different candidate generation behaviors depending on how often the model was reconstructed. This is undesirable as we want the model to resume generation rather than repeating from scratch.

Prior to this diff, UniformGenerator did not have init_position, so had this exact issue. We fix it here.

Differential Revision: D61622058

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

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

@codecov-commenter
Copy link

codecov-commenter commented Aug 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.29%. Comparing base (84b307d) to head (0b6a73e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2686      +/-   ##
==========================================
- Coverage   95.29%   95.29%   -0.01%     
==========================================
  Files         495      495              
  Lines       47832    47829       -3     
==========================================
- Hits        45582    45579       -3     
  Misses       2250     2250              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@facebook-github-bot
Copy link
Contributor

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

saitcakmak added a commit to saitcakmak/Ax that referenced this pull request Aug 21, 2024
Summary:
Pull Request resolved: facebook#2686

`SobolGenerator` uses `init_position` to ensure that when the model is reconstructed, it resumes candidate generation from where it was left (rather than starting from the beginning of the sequence). Without this, when `deduplicate=False`, the model would generate the same points it has already generated, which would lead to different candidate generation behaviors depending on how often the model was reconstructed. This is undesirable as we want the model to resume generation rather than repeating from scratch.

Prior to this diff, `UniformGenerator` did not have `init_position`, so had this exact issue. We fix it here.

Reviewed By: Balandat

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

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

saitcakmak added a commit to saitcakmak/Ax that referenced this pull request Aug 21, 2024
Summary:
Pull Request resolved: facebook#2686

`SobolGenerator` uses `init_position` to ensure that when the model is reconstructed, it resumes candidate generation from where it was left (rather than starting from the beginning of the sequence). Without this, when `deduplicate=False`, the model would generate the same points it has already generated, which would lead to different candidate generation behaviors depending on how often the model was reconstructed. This is undesirable as we want the model to resume generation rather than repeating from scratch.

Prior to this diff, `UniformGenerator` did not have `init_position`, so had this exact issue. We fix it here.

Reviewed By: Balandat

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

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

saitcakmak added a commit to saitcakmak/Ax that referenced this pull request Aug 21, 2024
Summary:
Pull Request resolved: facebook#2686

`SobolGenerator` uses `init_position` to ensure that when the model is reconstructed, it resumes candidate generation from where it was left (rather than starting from the beginning of the sequence). Without this, when `deduplicate=False`, the model would generate the same points it has already generated, which would lead to different candidate generation behaviors depending on how often the model was reconstructed. This is undesirable as we want the model to resume generation rather than repeating from scratch.

Prior to this diff, `UniformGenerator` did not have `init_position`, so had this exact issue. We fix it here.

Reviewed By: Balandat

Differential Revision: D61622058
saitcakmak and others added 2 commits August 22, 2024 09:22
Summary: Prior to this, if you compared a float with an ndarray, you'd get a boolean ndarray, which would lead to `ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()`.

Differential Revision: D61666276
Summary:
Pull Request resolved: facebook#2686

`SobolGenerator` uses `init_position` to ensure that when the model is reconstructed, it resumes candidate generation from where it was left (rather than starting from the beginning of the sequence). Without this, when `deduplicate=False`, the model would generate the same points it has already generated, which would lead to different candidate generation behaviors depending on how often the model was reconstructed. This is undesirable as we want the model to resume generation rather than repeating from scratch.

Prior to this diff, `UniformGenerator` did not have `init_position`, so had this exact issue. We fix it here.

Reviewed By: Balandat

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

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in e800973.

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.

3 participants