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

Remove status quo weight override from COPY_DB_IDS_ATTRS_TO_SKIP #2615

Closed

Conversation

danielcohenlive
Copy link

Summary:
Encoder and decoder both deal with _status_quo_weight_override. In decoder in happens through the status quo generator run. The problem is when a trial has a _status_quo_weight_override, but no status quo. The solution in this diff is to make it impossible (unless you use protected fields directly) to have a _status_quo_weight_override without a status_quo.

How could this be wrong?

If the user needs to store a status quo weight override on the trial for later but does not yet have a status quo. But I don't know why they could only calculate the weight now and not later.

Differential Revision: D60413211

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

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

@codecov-commenter
Copy link

codecov-commenter commented Jul 30, 2024

Codecov Report

Attention: Patch coverage is 97.22222% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.22%. Comparing base (8e07000) to head (90b3281).
Report is 1 commits behind head on main.

Files Patch % Lines
ax/core/batch_trial.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2615      +/-   ##
==========================================
- Coverage   95.22%   95.22%   -0.01%     
==========================================
  Files         492      492              
  Lines       47672    47692      +20     
==========================================
+ Hits        45394    45413      +19     
- Misses       2278     2279       +1     

☔ 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: D60413211

danielcohenlive pushed a commit to danielcohenlive/Ax-1 that referenced this pull request Jul 31, 2024
…ebook#2615)

Summary:
Pull Request resolved: facebook#2615

Encoder and decoder both deal with `_status_quo_weight_override`.  In decoder in happens through the status quo generator run.  The problem is when a trial has a `_status_quo_weight_override`, but no status quo.  The solution in this diff is to make it impossible (unless you use protected fields directly) to have a `_status_quo_weight_override` without a `status_quo`.

## How could this be wrong?
If the user needs to store a status quo weight override on the trial for later but does not yet have a status quo.  But I don't know why they could only calculate the weight now and not later.

Reviewed By: mgarrard

Differential Revision: D60413211
…ebook#2615)

Summary:
Pull Request resolved: facebook#2615

Encoder and decoder both deal with `_status_quo_weight_override`.  In decoder in happens through the status quo generator run.  The problem is when a trial has a `_status_quo_weight_override`, but no status quo.  The solution in this diff is to make it impossible (unless you use protected fields directly) to have a `_status_quo_weight_override` without a `status_quo`.

## How could this be wrong?
If the user needs to store a status quo weight override on the trial for later but does not yet have a status quo.  But I don't know why they could only calculate the weight now and not later.

Reviewed By: mgarrard

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

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in abacaea.

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