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

Deprecate weights argument of risk measures in favor of a preprocessing_function. #1400

Closed
wants to merge 1 commit into from

Conversation

saitcakmak
Copy link
Contributor

Summary:
Deprecates the weights argument of risk measures in favor of a preprocessing_function. This is superior in that it allows better modification of the samples before computing the risk measures.

This supports use cases such as filtering non-objective outcomes or applying feasibility weighting all within the risk measure itself. As a result, it helps avoid a number of if/else blocks when implementing robust optimization support in Ax.

Differential Revision: D39493308

@facebook-github-bot facebook-github-bot added CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported labels Sep 14, 2022
@facebook-github-bot
Copy link
Contributor

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

@codecov
Copy link

codecov bot commented Sep 14, 2022

Codecov Report

Merging #1400 (37adcf0) into main (d2117e2) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 37adcf0 differs from pull request most recent head 8588983. Consider uploading reports for the commit 8588983 to get more accurate results

@@            Coverage Diff            @@
##              main     #1400   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          124       124           
  Lines        11344     11354   +10     
=========================================
+ Hits         11344     11354   +10     
Impacted Files Coverage Δ
...tion/multi_objective/multi_output_risk_measures.py 100.00% <100.00%> (ø)
botorch/acquisition/risk_measures.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@Balandat Balandat left a comment

Choose a reason for hiding this comment

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

lgtm, couple of nits

if weights is not None:
warnings.warn(
"`weights` argument of risk measures is deprecated and will be removed "
" in a future version. Use a `preprocessing_function` instead.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
" in a future version. Use a `preprocessing_function` instead.",
"in a future version. Use a `preprocessing_function` instead.",

@@ -498,7 +518,7 @@ def test_set_baseline_Y(self):
mars.set_baseline_Y(model=model, X_baseline=X_baseline)
self.assertTrue(torch.equal(mars.baseline_Y, torch.tensor([[-0.5, -0.5]])))

def test_get_Y_normalization_bounds(self):
def fest_get_Y_normalization_bounds(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

ftest...

…ssing_function`. (pytorch#1400)

Summary:
Pull Request resolved: pytorch#1400

Deprecates the `weights` argument of risk measures in favor of a `preprocessing_function`. This is superior in that it allows better modification of the samples before computing the risk measures.

This supports use cases such as filtering non-objective outcomes or applying feasibility weighting all within the risk measure itself. As a result, it helps avoid a number of if/else blocks when implementing robust optimization support in Ax.

Reviewed By: Balandat

Differential Revision: D39493308

fbshipit-source-id: 1278cfe01031a45ebf71876dbbd0cce80b23fea0
@facebook-github-bot
Copy link
Contributor

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

saitcakmak added a commit to saitcakmak/botorch that referenced this pull request Sep 14, 2022
…ssing_function`. (pytorch#1400)

Summary:
Pull Request resolved: pytorch#1400

Deprecates the `weights` argument of risk measures in favor of a `preprocessing_function`. This is superior in that it allows better modification of the samples before computing the risk measures.

This supports use cases such as filtering non-objective outcomes or applying feasibility weighting all within the risk measure itself. As a result, it helps avoid a number of if/else blocks when implementing robust optimization support in Ax.

Differential Revision: https://internalfb.com/D39493308

fbshipit-source-id: 072d0a4c54de854106fa74db5e2ca2efa55d445a
facebook-github-bot pushed a commit that referenced this pull request Aug 28, 2023
…ureMCObjective` and `squeeze_last_dim` (#1994)

Summary:
## Motivation

* `utils.transforms.squeeze_last_dim` was deprecated prior to 0.7.0 (#487 ), so since we are at 0.9.2 it can be deleted.
* The `weights` argument of `acquisition.risk_measures.RiskMeasureMCObjective` was deprecated in 0.7.2 (#1400 ). Technically, the deprecation message only says that `weights` should be None rather than that it should not be passed, but I think it's okay to just remove it. Where it was not the last argument, I added a `*` to require that subsequent arguments be keyword-only.
*
### Have you read the [Contributing Guidelines on pull requests](https://github.com/pytorch/botorch/blob/main/CONTRIBUTING.md#pull-requests)?

Yes

Pull Request resolved: #1994

Test Plan:
Existing units

## Related PRs

#487, #1400

Reviewed By: Balandat

Differential Revision: D48738152

Pulled By: esantorella

fbshipit-source-id: 55bee2937e55b4d993c8d1cfc68330e1eb54a8ea
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants