-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add log(Pval) term to lognormal prior #558
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello Quang. Thanks for considering this issue. It cannot be accepted as is because it does not provide backward compatibility for all existing model applications. I also will want the logic and rationale of the proposed change to be documented more thoroughly.
My initial impression is to agree with this request from a statistical standpoint. When used as a prior, the pdf is a function of the state variable which in this case is some parameter say 'p'. The @quang-huynh 's code shows this, but another way to demonstrate it is to integrate the model. With no other sources of information the posterior will just be the prior. So I modified the example to get posteriors and compare to the analytical prior. See code below which produces the following plot. I think this could be wrapped into issue #553 which will overhaul the objective function calculations. This is a good example of why using Curious to hear if there are other considerations.
|
closing this PR and redirecting discussion to #553 |
The explanation from @Cole-Monnahan-NOAA is much better than my terse code! On the math side, if we take the log of the lognormal probability density function and drop constants, then |
I understand, but SS3 is rarely used in a Bayesian mode and the value of 0.5*square(log(parm)-logmu)/square(sd) is simply a component of the objective function. So, does the correct approach depend upon whether the model is operating in bayesian mode vs maximum posterior density mode? SS3 was designed as a MPD model and has never been thoroughly examined for bayesian use, as Cole (and Jim Thorson) has proposed for #553 |
No, I don't think the approach changes with MPD vs. MCMC. The current SS3 equation generates a larger prior mode and SD than what the user specifies (both my and Cole's figures show this). This has been my experience with a couple assessments where the posterior mean and MPD for natural mortality was much higher than the prior mean for what I thought would be a relatively uninformative data set.
|
I agree that it does not depend on how the objective function will be used. The fundamental problem is that it's putting a normal prior in log space, but the manual suggests it's a lognormal prior. So the user will get a different answer than the expect, even if they are optimizing. You can see this from my example where those independent parameters the MLE will just be the mode which you can visualize, and the current one (on the left) would give a biased MLE for the parameter without any data. I turned on estimation in the above example and got
The mode of this distribution (meanlog=1.1, meansd=1.12) is exp(mu-sd^2) = 0.8569292. So you can see that the estimate will be off. It's not convening the right information in the prior that the user expects. That's my interpretation at least. |
Concisely describe what has been changed/addressed in the pull request.
Add
log(pval)
to negative log prior for lognormal distributionWhat tests have been done?
Where are the relevant files?
What tests/review still need to be done?
Is there an input change for users to Stock Synthesis?
No input change.
Additional information (optional).
Current negative log prior is of the form
0.5 * square(log(x) - mu)
. Since x is estimated in normal space, an additional termlog(x)
from the lognormal distribution is needed.Some R code to verify: