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

LinearEllipticalSliceSampler Robustness Improvements #1859

Closed
wants to merge 1 commit into from

Conversation

SebastianAment
Copy link
Contributor

@SebastianAment SebastianAment commented Jun 5, 2023

Summary:
This commit improves the robustness of the linear elliptical slice sampler, primarily with two modifications:

  1. A rewrite of the computation of the angles of the ellipse that lead to intersections with the constraint boundaries that gets rid of the delta_theta parameter which cannot be set universally without sacrificing either correctness or causing errors due to floating point imprecisions.
  2. Contracting the feasible slices of the ellipse by an amount close to the numerical precision to guarantee that the resulting samples satisfy the constraints numerically.

The commit also introduces a high dimensional test case that enforces the monotonicity of adjacent elements of the sample vectors, which originally led to the discovery of all the issues that are fixed by the above steps.

Differential Revision: D46422349

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

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

@codecov
Copy link

codecov bot commented Jun 5, 2023

Codecov Report

Merging #1859 (5886fcd) into main (30eddaf) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 5886fcd differs from pull request most recent head 35cd148. Consider uploading reports for the commit 35cd148 to get more accurate results

@@            Coverage Diff            @@
##              main     #1859   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          172       172           
  Lines        15102     15111    +9     
=========================================
+ Hits         15102     15111    +9     
Impacted Files Coverage Δ
botorch/utils/probability/lin_ess.py 100.00% <100.00%> (ø)

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

SebastianAment added a commit to SebastianAment/botorch that referenced this pull request Jun 5, 2023
Summary:
Pull Request resolved: pytorch#1859

This commit improves the robustness of the linear elliptical slice sampler, primarily with two modifications:
1) A rewrite of the computation of the angles of the ellipse that lead to intersections with the constraint boundaries that gets ride of the `delta_theta` parameter which cannot be set universally without sacrificing either correctness or causing errors due to floating point imprecisions.
2) Contracting the feasible slices of the ellipse by an amount close to the numerical precision to guarantee that the resulting samples satisfy the constraints numerically.

The commit also introduces a high dimensional test case that enforces the monotonicity of adjacent elements of the sample vectors, which originally lead to the discovery of all the issues that are fixed by the above steps.

Differential Revision: D46422349

fbshipit-source-id: 5fc474975a5d798374b3d6c7e36f6b9cf9ae6f27
@facebook-github-bot
Copy link
Contributor

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

Summary:
Pull Request resolved: pytorch#1859

This commit improves the robustness of the linear elliptical slice sampler, primarily with two modifications:
1) A rewrite of the computation of the angles of the ellipse that lead to intersections with the constraint boundaries that gets rid of the `delta_theta` parameter which cannot be set universally without sacrificing either correctness or causing errors due to floating point imprecisions.
2) Contracting the feasible slices of the ellipse by an amount close to the numerical precision to guarantee that the resulting samples satisfy the constraints numerically.

The commit also introduces a high dimensional test case that enforces the monotonicity of adjacent elements of the sample vectors, which originally led to the discovery of all the issues that are fixed by the above steps.

Reviewed By: Balandat

Differential Revision: D46422349

fbshipit-source-id: 526d090d4360ab5b073d7684cfd31495541da866
@facebook-github-bot
Copy link
Contributor

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

SebastianAment added a commit to SebastianAment/botorch that referenced this pull request Jun 9, 2023
Summary:
Pull Request resolved: pytorch#1859

This commit improves the robustness of the linear elliptical slice sampler, primarily with two modifications:
1) A rewrite of the computation of the angles of the ellipse that lead to intersections with the constraint boundaries that gets rid of the `delta_theta` parameter which cannot be set universally without sacrificing either correctness or causing errors due to floating point imprecisions.
2) Contracting the feasible slices of the ellipse by an amount close to the numerical precision to guarantee that the resulting samples satisfy the constraints numerically.

The commit also introduces a high dimensional test case that enforces the monotonicity of adjacent elements of the sample vectors, which originally led to the discovery of all the issues that are fixed by the above steps.

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

fbshipit-source-id: c6fcf9da5f99871a5ff4291f1f698f200ec5c4c3
SebastianAment added a commit to SebastianAment/botorch that referenced this pull request Jun 9, 2023
Summary:
Pull Request resolved: pytorch#1859

This commit improves the robustness of the linear elliptical slice sampler, primarily with two modifications:
1) A rewrite of the computation of the angles of the ellipse that lead to intersections with the constraint boundaries that gets rid of the `delta_theta` parameter which cannot be set universally without sacrificing either correctness or causing errors due to floating point imprecisions.
2) Contracting the feasible slices of the ellipse by an amount close to the numerical precision to guarantee that the resulting samples satisfy the constraints numerically.

The commit also introduces a high dimensional test case that enforces the monotonicity of adjacent elements of the sample vectors, which originally led to the discovery of all the issues that are fixed by the above steps.

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

fbshipit-source-id: 88e85a566a963eef7403114e31c2e17f05ad1182
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in ef52ea9.

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.

2 participants