From 70425503dd4512a967c999bad26fcac695aedfbd Mon Sep 17 00:00:00 2001 From: Janos Gabler Date: Mon, 22 May 2023 10:29:11 +0200 Subject: [PATCH] Remove direct estimagic dependencies to avoid circular imports (#6) --- pyproject.toml | 1 + src/tranquilo/__init__.py | 4 ---- src/tranquilo/options.py | 2 +- src/tranquilo/process_arguments.py | 29 +++++++++++++++++++----- src/tranquilo/sample_points.py | 30 ++++++++++++++++--------- src/tranquilo/tranquilo.py | 21 ----------------- src/tranquilo/wrap_criterion.py | 23 +++++++++++++++++-- tests/subsolvers/test_gqtpar_lambdas.py | 18 ++++++++++++--- tests/test_fit_models.py | 2 +- tests/test_tranquilo.py | 28 +++++++++++++++++++---- tests/test_visualize.py | 27 ++++++++++++++++++++-- 11 files changed, 130 insertions(+), 55 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index b35ccb47..ed4bf3be 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -72,6 +72,7 @@ black = 1 filterwarnings = [ "ignore:delta_grad == 0.0", # UserWarning in test_poisedness.py "ignore:Jupyter is migrating", # DeprecationWarning from jupyter client + "ignore:Noisy scalar functions are experimental", ] markers = [ "wip: Tests that are work-in-progress.", diff --git a/src/tranquilo/__init__.py b/src/tranquilo/__init__.py index 2589aa8f..e69de29b 100644 --- a/src/tranquilo/__init__.py +++ b/src/tranquilo/__init__.py @@ -1,4 +0,0 @@ -from tranquilo.tranquilo import tranquilo, tranquilo_ls - - -__all__ = ["tranquilo", "tranquilo_ls"] diff --git a/src/tranquilo/options.py b/src/tranquilo/options.py index 72bad1a5..2a92516a 100644 --- a/src/tranquilo/options.py +++ b/src/tranquilo/options.py @@ -184,7 +184,7 @@ class FilterOptions(NamedTuple): class SamplerOptions(NamedTuple): distribution: str = None hardness: float = 1 - algorithm: str = "scipy_lbfgsb" + algorithm: str = "L-BFGS-B" algo_options: dict = None criterion: str = None n_points_randomsearch: int = 1 diff --git a/src/tranquilo/process_arguments.py b/src/tranquilo/process_arguments.py index d0c05240..d8c14d04 100644 --- a/src/tranquilo/process_arguments.py +++ b/src/tranquilo/process_arguments.py @@ -1,9 +1,5 @@ import numpy as np -from estimagic.optimization.algo_options import ( - CONVERGENCE_RELATIVE_CRITERION_TOLERANCE, - CONVERGENCE_RELATIVE_GRADIENT_TOLERANCE, -) from tranquilo.acceptance_decision import get_acceptance_decider from tranquilo.aggregate_models import get_aggregator from tranquilo.bounds import Bounds @@ -33,6 +29,7 @@ from tranquilo.sample_points import get_sampler from tranquilo.solve_subproblem import get_subsolver from tranquilo.wrap_criterion import get_wrapped_criterion +import warnings def process_arguments( @@ -51,8 +48,8 @@ def process_arguments( convergence_absolute_criterion_tolerance=0.0, convergence_absolute_gradient_tolerance=0.0, convergence_absolute_params_tolerance=0.0, - convergence_relative_criterion_tolerance=CONVERGENCE_RELATIVE_CRITERION_TOLERANCE, - convergence_relative_gradient_tolerance=CONVERGENCE_RELATIVE_GRADIENT_TOLERANCE, + convergence_relative_criterion_tolerance=2e-9, + convergence_relative_gradient_tolerance=1e-8, convergence_relative_params_tolerance=1e-8, convergence_min_trust_region_radius=0.0, # stopping options @@ -90,6 +87,26 @@ def process_arguments( infinity_handler="relative", residualize=None, ): + # warning for things that do not work well yet + if noisy and functype == "scalar": + msg = ( + "Noisy scalar functions are experimental and likely to give very " + "suboptimal results." + ) + warnings.warn(msg) + if noisy and n_cores > 1: + msg = ( + "Parallelization together with noisy functions is experimental and likely " + "to give very suboptimal results." + ) + warnings.warn(msg) + if n_cores > 1 and functype == "scalar": + msg = ( + "Parallelization together with scalar functions is experimental and likely " + "to give very suboptimal results." + ) + warnings.warn(msg) + # process convergence options conv_options = ConvOptions( disable=bool(disable_convergence), diff --git a/src/tranquilo/sample_points.py b/src/tranquilo/sample_points.py index 31ec21e7..5ff0745a 100644 --- a/src/tranquilo/sample_points.py +++ b/src/tranquilo/sample_points.py @@ -4,9 +4,10 @@ from scipy.spatial.distance import pdist from scipy.special import gammainc, logsumexp -import estimagic as em +from scipy.optimize import minimize, Bounds from tranquilo.get_component import get_component from tranquilo.options import SamplerOptions +import functools def get_sampler(sampler, user_options=None): @@ -241,8 +242,8 @@ def _optimal_hull_sampler( criterion = "determinant" if n_points == 1 else "distance" algo_options = {} if algo_options is None else algo_options - if "stopping_max_iterations" not in algo_options: - algo_options["stopping_max_iterations"] = 2 * n_params + 5 + if "maxiter" not in algo_options: + algo_options["maxiter"] = 2 * n_params + 5 if existing_xs is not None: # map existing points into unit space for easier optimization @@ -302,15 +303,14 @@ def _optimal_hull_sampler( if existing_xs_unit is None and n_points == 1: opt_params = x0 else: - res = em.maximize( - criterion=func_dict[criterion], - params=x0, - algorithm=algorithm, - lower_bounds=-np.ones_like(x0), - upper_bounds=np.ones_like(x0), - algo_options=algo_options, + res = minimize( + switch_sign(func_dict[criterion]), + x0, + method=algorithm, + bounds=Bounds(-np.ones_like(x0), np.ones_like(x0)), + options=algo_options, ) - opt_params = res.params + opt_params = res.x # Make sure the optimal sampling is actually better than the initial one with # respect to the fekete criterion. This could be violated if the surrogate @@ -464,3 +464,11 @@ def _project_onto_unit_hull(x, trustregion_shape): norm = np.linalg.norm(x, axis=1, ord=order).reshape(-1, 1) projected = x / norm return projected + + +def switch_sign(func): + @functools.wraps(func) + def wrapper(*args, **kwargs): + return -func(*args, **kwargs) + + return wrapper diff --git a/src/tranquilo/tranquilo.py b/src/tranquilo/tranquilo.py index ca1ee5e1..845d2bb9 100644 --- a/src/tranquilo/tranquilo.py +++ b/src/tranquilo/tranquilo.py @@ -1,10 +1,8 @@ import functools -from functools import partial from typing import NamedTuple import numpy as np -from estimagic.decorators import mark_minimizer from tranquilo.adjust_radius import adjust_radius from tranquilo.filter_points import ( drop_worst_points, @@ -513,25 +511,6 @@ def _is_converged(states, options): return converged, msg -tranquilo = mark_minimizer( - func=partial(_tranquilo, functype="scalar"), - name="tranquilo", - primary_criterion_entry="value", - needs_scaling=True, - is_available=True, - is_global=False, -) - -tranquilo_ls = mark_minimizer( - func=partial(_tranquilo, functype="least_squares"), - primary_criterion_entry="root_contributions", - name="tranquilo_ls", - needs_scaling=True, - is_available=True, - is_global=False, -) - - def _concatenate_indices(first, second): first = np.atleast_1d(first).astype(int) second = np.atleast_1d(second).astype(int) diff --git a/src/tranquilo/wrap_criterion.py b/src/tranquilo/wrap_criterion.py index 1a749623..705bf697 100644 --- a/src/tranquilo/wrap_criterion.py +++ b/src/tranquilo/wrap_criterion.py @@ -2,8 +2,6 @@ import numpy as np -from estimagic.batch_evaluators import process_batch_evaluator - def get_wrapped_criterion(criterion, batch_evaluator, n_cores, history): """Wrap the criterion function to do get parallelization and history handling. @@ -66,3 +64,24 @@ def wrapper_criterion(eval_info): ) return wrapper_criterion + + +def process_batch_evaluator(batch_evaluator="joblib"): + batch_evaluator = "joblib" if batch_evaluator is None else batch_evaluator + + if callable(batch_evaluator): + out = batch_evaluator + elif isinstance(batch_evaluator, str): + if batch_evaluator == "joblib": + from estimagic.batch_evaluators import joblib_batch_evaluator as out + elif batch_evaluator == "pathos": + from estimagic.batch_evaluators import pathos_mp_batch_evaluator as out + else: + raise ValueError( + "Invalid batch evaluator requested. Currently only 'pathos' and " + "'joblib' are supported." + ) + else: + raise TypeError("batch_evaluator must be a callable or string.") + + return out diff --git a/tests/subsolvers/test_gqtpar_lambdas.py b/tests/subsolvers/test_gqtpar_lambdas.py index b0d1243f..25284175 100644 --- a/tests/subsolvers/test_gqtpar_lambdas.py +++ b/tests/subsolvers/test_gqtpar_lambdas.py @@ -1,6 +1,18 @@ -import estimagic as em +from estimagic.optimization.optimize import minimize from estimagic.benchmarking.get_benchmark_problems import get_benchmark_problems -from tranquilo import tranquilo +from tranquilo.tranquilo import _tranquilo +from estimagic.decorators import mark_minimizer +from functools import partial + + +tranquilo = mark_minimizer( + func=partial(_tranquilo, functype="scalar"), + name="tranquilo", + primary_criterion_entry="value", + needs_scaling=True, + is_available=True, + is_global=False, +) def test_gqtpar_lambdas(): @@ -13,7 +25,7 @@ def test_gqtpar_lambdas(): } problem_info = get_benchmark_problems("more_wild")["freudenstein_roth_good_start"] - em.minimize( + minimize( criterion=problem_info["inputs"]["criterion"], params=problem_info["inputs"]["params"], algo_options=algo_options, diff --git a/tests/test_fit_models.py b/tests/test_fit_models.py index 54b07b87..9408ee5a 100644 --- a/tests/test_fit_models.py +++ b/tests/test_fit_models.py @@ -1,6 +1,6 @@ import numpy as np import pytest -from estimagic import first_derivative, second_derivative +from estimagic.differentiation.derivatives import first_derivative, second_derivative from tranquilo.fit_models import _quadratic_features, get_fitter from tranquilo.region import Region from numpy.testing import assert_array_almost_equal, assert_array_equal diff --git a/tests/test_tranquilo.py b/tests/test_tranquilo.py index 45f0a6b3..3a262fd2 100644 --- a/tests/test_tranquilo.py +++ b/tests/test_tranquilo.py @@ -3,11 +3,31 @@ import numpy as np import pytest from estimagic.optimization.optimize import minimize -from tranquilo.tranquilo import ( - tranquilo, - tranquilo_ls, -) +from tranquilo.tranquilo import _tranquilo from numpy.testing import assert_array_almost_equal as aaae +from estimagic.decorators import mark_minimizer +from functools import partial + + +tranquilo = mark_minimizer( + func=partial(_tranquilo, functype="scalar"), + name="tranquilo", + primary_criterion_entry="value", + needs_scaling=True, + is_available=True, + is_global=False, +) + + +tranquilo_ls = mark_minimizer( + func=partial(_tranquilo, functype="least_squares"), + primary_criterion_entry="root_contributions", + name="tranquilo_ls", + needs_scaling=True, + is_available=True, + is_global=False, +) + # ====================================================================================== # Test tranquilo end-to-end diff --git a/tests/test_visualize.py b/tests/test_visualize.py index 587f62d7..3162c442 100644 --- a/tests/test_visualize.py +++ b/tests/test_visualize.py @@ -1,7 +1,30 @@ import pytest -from estimagic import get_benchmark_problems, minimize +from estimagic.optimization.optimize import minimize +from estimagic.benchmarking.get_benchmark_problems import get_benchmark_problems from tranquilo.visualize import visualize_tranquilo -from tranquilo import tranquilo, tranquilo_ls +from tranquilo.tranquilo import _tranquilo +from estimagic.decorators import mark_minimizer +from functools import partial + + +tranquilo = mark_minimizer( + func=partial(_tranquilo, functype="scalar"), + name="tranquilo", + primary_criterion_entry="value", + needs_scaling=True, + is_available=True, + is_global=False, +) + + +tranquilo_ls = mark_minimizer( + func=partial(_tranquilo, functype="least_squares"), + primary_criterion_entry="root_contributions", + name="tranquilo_ls", + needs_scaling=True, + is_available=True, + is_global=False, +) cases = [] algo_options = {