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

Refactor test function test utils #1839

Closed
wants to merge 1 commit into from

Conversation

Balandat
Copy link
Contributor

Summary:
Refactors the test utils for botorch test functions to make things more modular.

Instead of subclassing the base test cases, we now have class Mixins for synthetic, constrained, and multi-objective problems. This means we can mix-and-match the test case as needed.

Further, BaseTestProblemTestCase now subclasses BotorchTestCase so that we don't need to subclass from BotorchTestCase separately for every test.

Finally, this flattens the TestMultiObjectiveProblems and TestConstrainedMultiObjectiveProblems test collections (the idea behind the test design is that we have separate test cases for each problem (potentially with different arguments) to better identify and group the tests, so this reestablishes that.

Differential Revision: D45969036

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

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

Balandat added a commit to Balandat/botorch that referenced this pull request May 18, 2023
Summary:
Pull Request resolved: pytorch#1839

Refactors the test utils for botorch test functions to make things more modular.

Instead of subclassing the base test cases, we now have class Mixins for synthetic, constrained, and multi-objective problems. This means we can mix-and-match the test case as needed.

This also flattens the `TestMultiObjectiveProblems` and `TestConstrainedMultiObjectiveProblems` test collections (the idea behind the test design is that we have separate test cases for each problem (potentially with different arguments) to better identify and group the tests, so this reestablishes that.

Differential Revision: D45969036

fbshipit-source-id: 227662f7914eba3e461a604b6ace592cc9fca227
@facebook-github-bot
Copy link
Contributor

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

Balandat added a commit to Balandat/botorch that referenced this pull request May 18, 2023
Summary:
Pull Request resolved: pytorch#1839

Refactors the test utils for botorch test functions to make things more modular.

Instead of subclassing the base test cases, we now have class Mixins for synthetic, constrained, and multi-objective problems. This means we can mix-and-match the test case as needed.

This also flattens the `TestMultiObjectiveProblems` and `TestConstrainedMultiObjectiveProblems` test collections (the idea behind the test design is that we have separate test cases for each problem (potentially with different arguments) to better identify and group the tests, so this reestablishes that.

Differential Revision: D45969036

fbshipit-source-id: 2b3c00813185b93bfd55874f34b330ed7280d52b
@facebook-github-bot
Copy link
Contributor

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

@codecov
Copy link

codecov bot commented May 18, 2023

Codecov Report

Merging #1839 (ced2f30) into main (70d0c63) will not change coverage.
The diff coverage is 100.00%.

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

@@            Coverage Diff            @@
##              main     #1839   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          170       170           
  Lines        14937     14939    +2     
=========================================
+ Hits         14937     14939    +2     
Impacted Files Coverage Δ
botorch/utils/testing.py 100.00% <100.00%> (ø)

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

@@ -93,7 +93,7 @@ def assertAllClose(
)


class BaseTestProblemBaseTestCase:
class BaseTestProblemTestCaseMixIn:

functions: List[BaseTestProblem]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
functions: List[BaseTestProblem]
functions: Sequence[BaseTestProblem] = []

Copy link
Member

@esantorella esantorella left a comment

Choose a reason for hiding this comment

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

The refactoring makes a lot of sense, thanks! See comments.

Balandat added a commit to Balandat/botorch that referenced this pull request May 19, 2023
Summary:
Pull Request resolved: pytorch#1839

Refactors the test utils for botorch test functions to make things more modular.

Instead of subclassing the base test cases, we now have class Mixins for synthetic, constrained, and multi-objective problems. This means we can mix-and-match the test case as needed.

This also flattens the `TestMultiObjectiveProblems` and `TestConstrainedMultiObjectiveProblems` test collections (the idea behind the test design is that we have separate test cases for each problem (potentially with different arguments) to better identify and group the tests, so this reestablishes that.

Reviewed By: esantorella

Differential Revision: D45969036

fbshipit-source-id: 7e9ec80ed93bab55e857761dd462545de44ceca9
@facebook-github-bot
Copy link
Contributor

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

Summary:
Pull Request resolved: pytorch#1839

Refactors the test utils for botorch test functions to make things more modular.

Instead of subclassing the base test cases, we now have class Mixins for synthetic, constrained, and multi-objective problems. This means we can mix-and-match the test case as needed.

This also flattens the `TestMultiObjectiveProblems` and `TestConstrainedMultiObjectiveProblems` test collections (the idea behind the test design is that we have separate test cases for each problem (potentially with different arguments) to better identify and group the tests, so this reestablishes that.

Reviewed By: esantorella

Differential Revision: D45969036

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

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 98b4194.

@Balandat Balandat deleted the export-D45969036 branch May 29, 2023 19:00
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