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 callable lookup mechanism #441

Merged
merged 32 commits into from
Dec 20, 2024
Merged

Refactor callable lookup mechanism #441

merged 32 commits into from
Dec 20, 2024

Conversation

AdrianSosic
Copy link
Collaborator

@AdrianSosic AdrianSosic commented Dec 4, 2024

This PR refactors the callable lookup mechanism in the simulation module, fixing one of the major limitations of the module and preparing it for multi-target simulation.

In particular:

  • The position-based approach relying on the order of input arguments to the callable is replaced with a proper dataframe-based approach, similar to other user-facing functionality elsewhere. That is, the callable now expects a dataframe containing columns corresponding to the search space parameters and returns a dataframe containing columns corresponding to the targets of the objective.
  • A arrays_to_dataframes decorator utility has been added that allows to conveniently construct such callables from array-based ones
  • The custom_analytical example has been renamed to custom_blackbox and was rewritten from scratch, using the new approach and fixing the existing issues.
  • Redundant examples have been removed.
  • The filter_df function has been fixed such that it now correctly handles the edge case of an empty filter.

@AdrianSosic AdrianSosic self-assigned this Dec 4, 2024
@AdrianSosic AdrianSosic marked this pull request as draft December 4, 2024 15:37
@AdrianSosic AdrianSosic marked this pull request as ready for review December 5, 2024 10:24
examples/Backtesting/botorch_analytical.py Outdated Show resolved Hide resolved
baybe/utils/dataframe.py Outdated Show resolved Hide resolved
baybe/simulation/lookup.py Outdated Show resolved Hide resolved
baybe/simulation/lookup.py Outdated Show resolved Hide resolved
examples/Backtesting/custom_analytical.py Outdated Show resolved Hide resolved
benchmarks/domains/synthetic_2C1D_1C.py Outdated Show resolved Hide resolved
benchmarks/domains/synthetic_2C1D_1C.py Outdated Show resolved Hide resolved
benchmarks/domains/synthetic_2C1D_1C.py Show resolved Hide resolved
examples/Basics/recommenders.py Show resolved Hide resolved
baybe/simulation/lookup.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@AVHopp AVHopp left a comment

Choose a reason for hiding this comment

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

I in general like the new approach. However, I am a bit puzzled by the amount of deleted examples.

baybe/simulation/lookup.py Show resolved Hide resolved
examples/Backtesting/custom_analytical.py Outdated Show resolved Hide resolved
baybe/simulation/lookup.py Outdated Show resolved Hide resolved
baybe/simulation/lookup.py Outdated Show resolved Hide resolved
baybe/utils/botorch_wrapper.py Outdated Show resolved Hide resolved
examples/Basics/recommenders.py Show resolved Hide resolved
baybe/simulation/lookup.py Outdated Show resolved Hide resolved
baybe/simulation/lookup.py Outdated Show resolved Hide resolved
benchmarks/domains/synthetic_2C1D_1C.py Outdated Show resolved Hide resolved
examples/Backtesting/custom_analytical.py Outdated Show resolved Hide resolved
The callable now operates with dataframe input/output, replacing the
positional indexing approach with a clean label-based approach using
parameter and target names.
Will be replaced with a proper one showing the difference between
single target, desirability, and Pareto optimization.
The current version provides minimal additional insights compared to what is already presented in `custom_analytical.py`. Instead of just reiterating the same logic, it should rather compare different hybrid optimizers. For now, it is dropped because refactoring is not worth it.
Copy link
Collaborator

@AVHopp AVHopp left a comment

Choose a reason for hiding this comment

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

Like it a lot. Some things could still be improved though :)

baybe/utils/dataframe.py Outdated Show resolved Hide resolved
docs/userguide/simulation.md Show resolved Hide resolved
docs/userguide/simulation.md Outdated Show resolved Hide resolved
docs/userguide/simulation.md Show resolved Hide resolved
docs/userguide/simulation.md Show resolved Hide resolved
examples/Backtesting/custom_analytical.py Outdated Show resolved Hide resolved
examples/Backtesting/custom_analytical.py Outdated Show resolved Hide resolved
examples/Constraints_Continuous/hybrid_space.py Outdated Show resolved Hide resolved
examples/Searchspaces/continuous_space.py Show resolved Hide resolved
examples/Transfer_Learning/backtesting.py Outdated Show resolved Hide resolved
baybe/utils/dataframe.py Show resolved Hide resolved
baybe/simulation/lookup.py Outdated Show resolved Hide resolved
baybe/simulation/lookup.py Outdated Show resolved Hide resolved
AdrianSosic and others added 2 commits December 19, 2024 13:20
Co-authored-by: Martin Fitzner <17951239+Scienfitz@users.noreply.github.com>
Copy link
Collaborator

@AVHopp AVHopp left a comment

Choose a reason for hiding this comment

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

Approve to unblock in case you want to get this done over the holidays

@AdrianSosic AdrianSosic force-pushed the refactor/lookup branch 2 times, most recently from 6a7d0ca to 21f611c Compare December 20, 2024 09:33
@AdrianSosic AdrianSosic merged commit fd78099 into main Dec 20, 2024
11 checks passed
@AdrianSosic AdrianSosic deleted the refactor/lookup branch December 20, 2024 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants