Repository: stan-dev/cmdstanpy

Issue:
variational output shouldn't throw with when ADVI fails to converge
#### Summary:

I ran a model with ADVI and got:

```
>>> fit = mod.variational()
INFO:cmdstanpy:start chain 1
INFO:cmdstanpy:finish chain 1
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/bbales2/.local/lib/python3.8/site-packages/cmdstanpy/model.py", line 1079, in variational
    raise RuntimeError('The algorithm may not have converged.')
RuntimeError: The algorithm may not have converged.
```

ADVI will fail to converge a lot and the intermediate output is useful in figuring out what went wrong. This can be a warning but shouldn't be an error.

Model was:
```stan
parameters {
    real x1;
    vector[2] x2[2];
    real x3[2, 2, 2, 2];
}

model {
    x2[1] ~ normal(5, 1);
    x2[2] ~ normal(0, 1);
    x1 ~ normal(0, 1);
    to_array_1d(x3) ~ normal(0, 1);
}

generated quantities {
    int z1 = bernoulli_rng(0.5);
}
```


Discussion:

Patch:
diff --git a/cmdstanpy/compiler_opts.py b/cmdstanpy/compiler_opts.py
index adefa0a..46c2414 100644
--- a/cmdstanpy/compiler_opts.py
+++ b/cmdstanpy/compiler_opts.py
@@ -40,6 +40,14 @@ STANC_IGNORE_OPTS = [
     'version',
 ]
 
+CPP_OPTS = [
+    'STAN_OPENCL',
+    'OPENCL_DEVICE_ID',
+    'OPENCL_PLATFORM_ID',
+    'STAN_MPI',
+    'STAN_THREADS',
+]
+
 
 class CompilerOptions:
     """
@@ -143,10 +151,18 @@ class CompilerOptions:
         """
         if self._cpp_options is None:
             return
-        for key in ['OPENCL_DEVICE_ID', 'OPENCL_PLATFORM_ID']:
-            if key in self._cpp_options:
-                self._cpp_options['STAN_OPENCL'] = 'TRUE'
-                val = self._cpp_options[key]
+        if (
+            'OPENCL_DEVICE_ID' in self._cpp_options.keys()
+            or 'OPENCL_PLATFORM_ID' in self._cpp_options.keys()
+        ):
+            self._cpp_options['STAN_OPENCL'] = 'TRUE'
+
+        for key, val in self._cpp_options.items():
+            if key not in CPP_OPTS:
+                raise ValueError(
+                    'unknown CmdStan makefile option: {}'.format(key)
+                )
+            if key in ['OPENCL_DEVICE_ID', 'OPENCL_PLATFORM_ID']:
                 if not isinstance(val, int) or val < 0:
                     raise ValueError(
                         '{} must be a non-negative integer value,'
diff --git a/cmdstanpy/model.py b/cmdstanpy/model.py
index 2cad977..67a7d61 100644
--- a/cmdstanpy/model.py
+++ b/cmdstanpy/model.py
@@ -336,21 +336,6 @@ class CmdStanModel:
                         get_logger().error(
                             'file %s, exception %s', stan_file, str(e)
                         )
-                        if 'PCH file' in str(e):
-                            get_logger().warning(
-                                "%s, %s",
-                                "CmdStan's precompiled header (PCH) files ",
-                                "may need to be rebuilt.",
-                            )
-                            get_logger().warning(
-                                "%s %s",
-                                "If your model failed to compile please run ",
-                                "install_cmdstan(overwrite=True).",
-                            )
-                            get_logger().warning(
-                                "If the issue persists please open a bug report"
-                            )
-
                         compilation_failed = True
 
                 if not compilation_failed:
@@ -1157,8 +1142,19 @@ class CmdStanModel:
             errors = re.findall(pat, contents)
             if len(errors) > 0:
                 valid = False
-        if require_converged and not valid:
-            raise RuntimeError('The algorithm may not have converged.')
+        if not valid:
+            if require_converged:
+                raise RuntimeError(
+                    'The algorithm may not have converged.\n'
+                    'If you would like to inspect the output, '
+                    're-call with require_converged=False'
+                )
+            # else:
+            get_logger().warning(
+                '%s\n%s',
+                'The algorithm may not have converged.',
+                'Proceeding because require_converged is set to False',
+            )
         if not runset._check_retcodes():
             msg = 'Error during variational inference:\n{}'.format(
                 runset.get_err_msgs()


Test patch:
diff --git a/test/test_compiler_opts.py b/test/test_compiler_opts.py
index 0517d7b..8b46912 100644
--- a/test/test_compiler_opts.py
+++ b/test/test_compiler_opts.py
@@ -121,6 +121,11 @@ class CompilerOptsTest(unittest.TestCase):
         opts.validate()
         self.assertEqual(opts.compose(), ['STAN_MPI=TRUE'])
 
+        cpp_opts['STAN_BAD'] = 'TRUE'
+        opts = CompilerOptions(cpp_options=cpp_opts)
+        with self.assertRaises(ValueError):
+            opts.validate()
+
     def test_opts_cpp_opencl(self):
         cpp_opts = {'OPENCL_DEVICE_ID': 1}
         opts = CompilerOptions(cpp_options=cpp_opts)
diff --git a/test/test_variational.py b/test/test_variational.py
index 2debade..581a01d 100644
--- a/test/test_variational.py
+++ b/test/test_variational.py
@@ -235,12 +235,22 @@ class VariationalTest(unittest.TestCase):
         )
         model = CmdStanModel(stan_file=stan)
         with self.assertRaisesRegex(
-            RuntimeError, 'algorithm may not have converged'
+            RuntimeError,
+            r'algorithm may not have converged\.\n.*require_converged',
         ):
             model.variational(algorithm='meanfield', seed=12345)
 
-        model.variational(
-            algorithm='meanfield', seed=12345, require_converged=False
+        with LogCapture() as log:
+            model.variational(
+                algorithm='meanfield', seed=12345, require_converged=False
+            )
+        log.check_present(
+            (
+                'cmdstanpy',
+                'WARNING',
+                'The algorithm may not have converged.\n'
+                'Proceeding because require_converged is set to False',
+            )
         )