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

Reorganize runner tests following previous class renaming #2991

Closed
wants to merge 3 commits into from

Conversation

esantorella
Copy link
Contributor

Summary:
See D64969707 for context.

This diff:

  • Moves the parts of tests/runners/test_botorch_test_problem that pertain to the runner into tests/test_benchmark_runner.py
  • Moves the parts that pertain to BoTorchTestFunction to tests/benchmark_test_functions/test_botorch_test_function.py
  • Moves tests/runners/test_surrogate_runner.py to tests/benchmark_test_functions/test_surrogate_test_function.py

Differential Revision: D65090663

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

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

@facebook-github-bot
Copy link
Contributor

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

esantorella added a commit to esantorella/Ax that referenced this pull request Oct 30, 2024
)

Summary:
Pull Request resolved: facebook#2991

See D64969707 for context.

This diff:
* Moves the parts of `tests/runners/test_botorch_test_problem` that pertain to the runner into `tests/test_benchmark_runner.py`
* Moves the parts that pertain to `BoTorchTestFunction` to `tests/benchmark_test_functions/test_botorch_test_function.py`
* Moves `tests/runners/test_surrogate_runner.py` to `tests/benchmark_test_functions/test_surrogate_test_function.py`

Differential Revision: D65090663
@codecov-commenter
Copy link

codecov-commenter commented Oct 30, 2024

Codecov Report

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

Project coverage is 95.61%. Comparing base (3aaca44) to head (05c52e2).

Files with missing lines Patch % Lines
ax/benchmark/benchmark_test_function.py 90.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2991   +/-   ##
=======================================
  Coverage   95.61%   95.61%           
=======================================
  Files         483      485    +2     
  Lines       48970    48977    +7     
=======================================
+ Hits        46821    46828    +7     
  Misses       2149     2149           

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

esantorella and others added 3 commits October 30, 2024 15:05
Summary:
Pull Request resolved: facebook#2977

** Context**
The current structure of the code is such:
* Each `BenchmarkProblem` has a `BenchmarkRunner`
* ` BenchmarkRunner` is the only runner
* A` BenchmarkRunner` has a `ParamBasedTestProblem`, which is either a `BoTorchTestProblem`, `SurrogateTestFunction`, or a special subclass such as `Jenatton`.
The directory structure and names have gotten quite out of touch with the code.

**New class names**
* `ParamBasedTestProblem` -> `TestFunction` (maybe we should call this `BenchmarkTestFunction`?)
* `BoTorchTestProblem` -> `BoTorchTestFunction`

**New directory structure**
| benchmark_problem.py
| problems/
|    | synthetic/hss/jenatton.py
|    | ...
| benchmark_runner.py
| test_function.py
| test_function.py
| test_functions/
|    | botorch_test.py
|    | surrogate.py

Future diffs:
* rename `BenchmarkRunner.test_problem` to `BenchmarkRunner.test_function` (D65088791)

Differential Revision: D64969707

Reviewed By: saitcakmak, Balandat
…n`, because it is a `BenchmarkTestFunction`

Summary: Rename `test_problem` attribute of `BenchmarkRunner` to `test_function`, because it is a `BenchmarkTestFunction`

Differential Revision: D65088791
)

Summary:
Pull Request resolved: facebook#2991

See D64969707 for context.

This diff:
* Moves the parts of `tests/runners/test_botorch_test_problem` that pertain to the runner into `tests/test_benchmark_runner.py`
* Moves the parts that pertain to `BoTorchTestFunction` to `tests/benchmark_test_functions/test_botorch_test_function.py`
* Moves `tests/runners/test_surrogate_runner.py` to `tests/benchmark_test_functions/test_surrogate_test_function.py`

Reviewed By: Balandat

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

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 46fa5a5.

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