Closed iantaylor-NOAA closed 3 months ago
Note: need to update Simple example to use do_recdev = 2 in order to pass the run-ss3-mcmc github action.
@iantaylor-NOAA do you think that we should update all Simple_xxx models to use do_recdev 2?
Good catch, thanks for fixing.
This PR adds an error when you try to run MCMC with the setting do_recdev = 1. It was inspired by this discussion https://github.com/nmfs-ost/ss3-source-code/discussions/566 where a user noted getting bad results in the MCMC which I think are driven by the bug in ADMB described at https://github.com/admb-project/admb/issues/107.
Concisely describe what has been changed/addressed in the pull request.
I thought of adding a warning instead of an error, but users may look at the warning file while running MLE estimates, decide that they can ignore the warnings, and not see that a new warning appears when running MCMC. I think the results of the MCMC are incorrect if using recdev option 1 so it's probably better to not allow them to get calculated in the first place.
What tests have been done?
I ran a model using
ss3 -mcmc 100
and confirmed that the command line and warning file includes the following message (feel free to suggest changes to it):Warning 1 Fatal Error! do_recdev option 1=devvector should not be used with MCMC, recommend option 2=simple deviations. For more detail see https://github.com/admb-project/admb/issues/107.
What tests/review still need to be done?
None.
Is there an input change for users to Stock Synthesis?
Additional context:
The build-ss3 github action is failing due to some issue with installing docker for the mac-os which seems unrelated to this change so probably doesn't need to hold up progress on this PR.