Fides 0.7.2 (#47)

* Verify options are of correct type (#45) * Verify options are of correct type Wasn't checked so far and led to very uninformative errors if wrong options were passed. * Apply suggestions from code review Co-authored-by: Fabian Fröhlich <[email protected]> * fix runtime warning for unbouned vars, fixes #46 * fixup * fix refined step * bump version Co-authored-by: Daniel Weindl <[email protected]>
fides-dev · Nov 26, 2021 · 4a666b5 · 4a666b5
1 parent db158b4
commit 4a666b5
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 12 deletions.
diff --git a/fides/minimize.py b/fides/minimize.py
@@ -22,7 +22,8 @@
 )
 from .logging import create_logger
 from collections import defaultdict
-
+from numbers import Real, Integral
+from pathlib import PosixPath, WindowsPath
 from typing import Callable, Dict, Optional, Tuple, Union, List
 
 
@@ -204,11 +205,7 @@ def __init__(self,
         if options is None:
             options = {}
 
-        for option in options:
-            try:
-                Options(option)
-            except ValueError:
-                raise ValueError(f'{option} is not a valid options field.')
+        validate_options(options)
 
         self.options: Dict = options
 
@@ -637,7 +634,8 @@ def get_affine_scaling(self) -> Tuple[np.ndarray, np.ndarray]:
         # this implements scaling for variables that are constrained by
         # bounds ( i and ii in Definition 2) bounds is equal to lb if grad <
         # 0 ub if grad >= 0
-        bounds = ((1 + v)*self.lb + (1 - v)*self.ub)/2
+        bounds = self.lb.copy()
+        bounds[self.grad >= 0] = self.ub[self.grad >= 0]
         bounded = np.isfinite(bounds)
         v[bounded] = self.x[bounded] - bounds[bounded]
         dv[bounded] = 1
@@ -846,3 +844,38 @@ def get_option(self, option):
 def _min_max_evs(mat: np.ndarray):
     evs = np.linalg.eigvals(mat)
     return np.real(np.min(evs)), np.real(np.max(evs))
+
+
+def validate_options(options: Dict):
+    """Check if the chosen options are valid"""
+    expected_types = {
+        Options.MAXITER: Integral,
+        Options.MAXTIME: Real,
+        Options.FATOL: Real,
+        Options.FRTOL: Real,
+        Options.XTOL: Real,
+        Options.GATOL: Real,
+        Options.GRTOL: Real,
+        Options.SUBSPACE_DIM: SubSpaceDim,
+        Options.STEPBACK_STRAT: StepBackStrategy,
+        Options.THETA_MAX: Real,
+        Options.DELTA_INIT: Real,
+        Options.MU: Real,
+        Options.ETA: Real,
+        Options.GAMMA1: Real,
+        Options.GAMMA2: Real,
+        Options.HISTORY_FILE: (str, PosixPath, WindowsPath),
+    }
+    for option_key, option_value in options.items():
+        try:
+            option = Options(option_key)
+        except ValueError:
+            raise ValueError(f'{option_key} is not a valid options field.')
+
+        expected_type = expected_types[option]
+        if not isinstance(option_value, expected_type):
+            if expected_type == Integral and int(option_value) == option_value:
+                continue
+            raise TypeError(f'Type mismatch for option {option_key}. '
+                            f'Expected {expected_type} but got '
+                            f'{type(option_value)}')
diff --git a/fides/steps.py b/fides/steps.py
@@ -496,8 +496,8 @@ def __init__(self, x, sg, hess, scaling, g_dscaling, delta, theta,
             ),
             LinearConstraint(
                 A=self.subspace,
-                lb=(lb - x) / scaling.diagonal(),
-                ub=(ub - x) / scaling.diagonal()
+                lb=self.theta * (lb - x) / scaling.diagonal(),
+                ub=self.theta * (ub - x) / scaling.diagonal()
             )
         ]
         self.guess: np.ndarray = step.sc.copy()

diff --git a/fides/trust_region.py b/fides/trust_region.py
@@ -114,7 +114,8 @@ def trust_region(x: np.ndarray,
                 lb
             ))
 
-        if stepback_strategy == StepBackStrategy.REFINE:
+        if stepback_strategy == StepBackStrategy.REFINE and \
+                tr_step.subspace.shape[1] > 1:
             ref_step = RefinedStep(
                 x, sg, hess, scaling, g_dscaling, delta, theta, ub, lb,
                 tr_step

diff --git a/fides/version.py b/fides/version.py
@@ -1 +1 @@
-__version__ = "0.7.1"
+__version__ = "0.7.2"
diff --git a/tests/test_minimize.py b/tests/test_minimize.py
@@ -1,6 +1,6 @@
 from fides import (
     Optimizer, BFGS, SR1, DFP, BB, BG, Broyden, GNSBFGS, HybridFixed,
-    HybridFraction, FX, SSM, TSSM, SubSpaceDim, StepBackStrategy
+    HybridFraction, FX, SSM, TSSM, SubSpaceDim, StepBackStrategy, Options
 )
 import numpy as np
 
@@ -377,3 +377,8 @@ def test_wrong_options():
             fun, ub=ub, lb=lb, verbose=logging.INFO,
             options={'option_doesnt_exist': 1}
         )
+    with pytest.raises(TypeError):
+        Optimizer(
+            fun, ub=ub, lb=lb, verbose=logging.INFO,
+            options={Options.FATOL: 'not a number'}
+        )