add fatal error when using MCMC with recdev option 1 #590
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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? |
- inspired by discussion: #566
e-perl-NOAA
force-pushed
the
mcmc-recdev-error
branch
from
9b71092
to
5de74be
Compare
Rick-Methot-NOAA
approved these changes
May 22, 2024
|
Good catch, thanks for fixing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds an error when you try to run MCMC with the setting do_recdev = 1.
It was inspired by this discussion #566 where a user noted getting bad results in the MCMC which I think are driven by the bug in ADMB described at admb-project/admb#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 100and 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.