Repository: wichmann-lab/python-psignifit

Issue:
Enhance test for fixed parameters
Fixed parameters can at the moment be set to non-sensical values, for example `width=0`, which results in some random exceptions along the line due to infinities or NaNs appearing everywhere. We need to catch this early on, ideally right in the `Configuration` object and fail with a proper error message.

Discussion:

Solution patch:
diff --git a/docs/_config.yml b/docs/_config.yml
index 62c788c..b9a98ae 100644
--- a/docs/_config.yml
+++ b/docs/_config.yml
@@ -8,6 +8,7 @@ copyright: "2024. Licence: GPLv2"  # Copyright year to be placed in the footer
 
 execute:
   execute_notebooks: force
+  timeout: -1
 
 #only_build_toc_files        : true
 
@@ -51,6 +52,7 @@ sphinx:
      "member-order": 'bysource',     # Order members as in source file.
     }
     exclude_patterns: ['_build']
+    nb_execution_show_tb: True
     intersphinx_mapping:
         python:
           - 'https://docs.python.org/3/'
diff --git a/psignifit/_configuration.py b/psignifit/_configuration.py
index 5008d8d..4385c5a 100644
--- a/psignifit/_configuration.py
+++ b/psignifit/_configuration.py
@@ -131,8 +131,36 @@ class Configuration:
             vkeys = _PARAMETERS
             if vkeys < set(value.keys()):
                 raise PsignifitException(
-                    f'Option fixed_paramters keys must be in {vkeys}. Given {list(value.keys())}!'
+                    f'Option fixed_parameters keys must be in {vkeys}. Given {list(value.keys())}!'
                 )
+            # Check that the fixed parameters have been set to meaningful values
+            for parm_name, parm_value in value.items():
+                # ERRORS
+                prefix = f'Fixed parameter {parm_name} must be strictly'
+                suffix = f': got {parm_name}={parm_value} instead!'
+                if parm_name == 'width' and parm_value <= 0:
+                    # 0 < width < +inf
+                    raise PsignifitException(f'{prefix} > 0 {suffix}')
+                elif parm_name == 'eta' and not(0 <= parm_value <= 1):
+                    # 0 <= eta <= 1
+                    raise PsignifitException(f'{prefix} 0 <= eta <= 1 {suffix}')
+                elif parm_name in ('gamma', 'lambda') and not(0 <= parm_value < 1):
+                    # 0 <= gamma < 1 and 0 <= lambda < 1
+                    raise PsignifitException(f'{prefix} 0 <= {parm_name} < 1 {suffix}')
+                # WARNINGS
+                if parm_name == 'lambda' and parm_value > 0.2:
+                    # lambda > 0.2 is unusual
+                    warnings.warn(f"You have fixed the lapse rate lambda to an unusually high value ({parm_value}). "
+                                  f"In typical psychophysical experiments lambda is rarely above 0.2 for human observers.")
+
+            if 'gamma' in value and 'lambda' in value:
+                gamma, lambda_ = value['gamma'], value['lambda']
+                # this condition can only happen if both lambda and gamma are fixed
+                # 0 <= gamma + lambda < 1
+                if not (0 <= gamma+lambda_ < 1):
+                    raise PsignifitException(f'For gamma and lambda the condition'
+                                             f' 0 <= gamma + lambda < 1 must always apply:'
+                                             f' got gamma={gamma}, lambda={lambda_} instead!')
 
 
     def check_experiment_type_matches_fixed_parameters(self, fixed_params, experiment_type):
@@ -140,7 +168,7 @@ class Configuration:
             if fixed_params is not None and 'gamma' in fixed_params:
                 warnings.warn(
                     f'The parameter gamma was fixed to {fixed_params["gamma"]}. In {ExperimentType.N_AFC.value} experiments gamma is automatically fixed to 1/n. Ignoring fixed gamma.')
-        if experiment_type == ExperimentType.EQ_ASYMPTOTE.value:
+        elif experiment_type == ExperimentType.EQ_ASYMPTOTE.value:
             if fixed_params is not None:
                 if 'gamma' in fixed_params:
                     pass # we assume the user knows what they are doing
@@ -152,6 +180,12 @@ class Configuration:
                             f'The parameters lambda {fixed_params["lambda"]} and'
                             f' gamma {fixed_params["gamma"]} were fixed to different values.'
                             f' In {experiment_type} experiments gamma and lambda need to be equal. ')
+        elif experiment_type == ExperimentType.YES_NO.value and fixed_params is not None:
+            if (gamma := fixed_params.get('gamma')) and gamma > 0.2:
+                    # gamma > 0.2 for yes/no experiments is unusual
+                    warnings.warn(f"You have fixed the guess rate gamma to an unusually high value ({gamma}). "
+                                  f"In typical psychophysical experiment of type 'yes/no' gamma is "
+                                  f"rarely above 0.2 for human observers.")
 
 
     def check_experiment_type(self, value):


Test patch:
diff --git a/tests/test_configuration.py b/tests/test_configuration.py
index 0eaa029..a2fffdd 100644
--- a/tests/test_configuration.py
+++ b/tests/test_configuration.py
@@ -1,11 +1,14 @@
 import dataclasses
 from unittest.mock import patch
 
+import numpy as np
 import pytest
 
 from psignifit._configuration import Configuration
 from psignifit._utils import PsignifitException
 
+EPSNEG = -np.finfo(np.float64).epsneg
+EPSPOS = np.finfo(np.float64).eps
 
 def test_setting_valid_option():
     c = Configuration(verbose=20)
@@ -138,3 +141,51 @@ def test_warning_for_equal_asymptote_fixing_lambda_and_gamma():
         Configuration(**options)
 
 
+def test_fixed_parm_width():
+    # 0 < width < +inf
+    with pytest.raises(PsignifitException, match='width'):
+        Configuration(fixed_parameters={'width': 0})
+    with pytest.raises(PsignifitException, match='width'):
+        Configuration(fixed_parameters={'width': EPSNEG})
+
+
+def test_fixed_parm_eta():
+    # 0 <= eta <= 1
+    with pytest.raises(PsignifitException, match='eta'):
+        Configuration(fixed_parameters={'eta': EPSNEG})
+    with pytest.raises(PsignifitException, match='eta'):
+        Configuration(fixed_parameters={'eta': 1+EPSPOS})
+
+
+def test_fixed_parm_gamma():
+    # 0 <= gamma < 1
+    with pytest.raises(PsignifitException, match='gamma'):
+        Configuration(fixed_parameters={'gamma': EPSNEG})
+    with pytest.raises(PsignifitException, match='gamma'):
+        Configuration(fixed_parameters={'gamma': 1})
+    # gamma > 0.2 and yes/no is unusual
+    with pytest.warns(UserWarning, match='unusual'):
+        Configuration(fixed_parameters={'gamma': 0.2+EPSPOS}, experiment_type='yes/no')
+
+
+def test_fixed_parm_lambda():
+    # 0 <= lambda < 1
+    with pytest.raises(PsignifitException, match='lambda'):
+        Configuration(fixed_parameters={'lambda': EPSNEG})
+    with pytest.raises(PsignifitException, match='lambda'):
+        Configuration(fixed_parameters={'lambda': 1})
+    # lambda > 0.2 is unusual
+    with pytest.warns(UserWarning, match='unusual'):
+        Configuration(fixed_parameters={'lambda': 0.2+EPSPOS})
+
+
+def test_fixed_parm_gamma_lambda():
+    # 0 <= gamma + lambda < 1
+    with pytest.raises(PsignifitException, match=r'gamma \+ lambda'):
+        Configuration(fixed_parameters={'gamma': 0.9, 'lambda': 0.2})
+    # gamma == 0 and lambda == 2 should fail!
+    with pytest.raises(PsignifitException):
+        Configuration(fixed_parameters={'gamma': 0, 'lambda': 2})
+    # gamma == 2 and lambda == 0 should fail!
+    with pytest.raises(PsignifitException):
+        Configuration(fixed_parameters={'gamma': 2, 'lambda': 0})
diff --git a/tests/test_numpy_scalars.py b/tests/test_numpy_scalars.py
index 7d11c93..38cb14b 100644
--- a/tests/test_numpy_scalars.py
+++ b/tests/test_numpy_scalars.py
@@ -36,7 +36,7 @@ def test_py_not_np_scalar_ci(input_data, ci_method):
 
 @pytest.mark.parametrize('fixed_parm', [(None, None),
                                         ('lambda', 3e-7),
-                                        ('gamma', 0.5),
+                                        ('gamma', 0.1),
                                         ('eta', 1e-4),
                                         ('threshold', 0.0046),
                                         ('width', 0.0046)])
diff --git a/tests/test_param_recovery.py b/tests/test_param_recovery.py
index 90682f8..3be2091 100644
--- a/tests/test_param_recovery.py
+++ b/tests/test_param_recovery.py
@@ -115,7 +115,10 @@ def test_parameter_recovery_fixed_params(fixed_param):
     # we fix it to a slightly off value, so we can check if stays fixed
     options['fixed_parameters'][fixed_param] = sim_params[fixed_param]
 
-    res = psignifit(data, **options)
+    with warnings.catch_warnings():
+        # ignore warning about gamma behind unusual
+        warnings.simplefilter("ignore")
+        res = psignifit(data, **options)
     # check fixed params are not touched
     assert res.parameter_estimate_MAP[fixed_param] == sim_params[fixed_param]