variational output shouldn't throw with when ADVI fails to converge

[bbbales2]

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:

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);
}

[WardBrian]

Is this not addressed by require_converged=False? Should we still be outputting a warning in that case, just not an error?

[mitzimorris]

yes, absolutely correct. require_converged was added in this PR: #283

perhaps adding more to the docs for variational? in which case, this could be done as part of documentation fix,
currently on branch: https://github.com/stan-dev/cmdstanpy/tree/docs/update-api-docs-405-409

[WardBrian]

I think documentation is necessary, I don't believe this argument is listed anywhere outside the docstring. If we want to emit a warning when the flag is set I can fire off a PR for that, will be a 2 line change and maybe an additional test.

[mitzimorris]

you mean, if require_converged == True, and fails to converge, then change error message so that it mentions setting require_converged to false?

cmdstanpy/cmdstanpy/model.py

Lines 1163 to 1164 in 0c79b50

	msg = 'Error during variational inference:\n{}'.format(
	runset.get_err_msgs()

that would be far more effective than documentation. yes, please do!

[WardBrian]

That’s half my suggestion:

1. Change the message in the ValueError to mention require_convergence
2. If require_convergence is False and the model fails, emit a logging message about this (to ensure someone doesn’t set the flag and then forget, proceeding as if it converged)

Sound reasonable?

[mitzimorris]

extremely reasonable!