search

Pacific-salmon-assess / samSim

3 stars 0 forks source link

arSigTransform correction #29

Closed catarinawor closed 1 year ago

catarinawor commented 1 year ago

I am wondering if the arSigTransform option should be adjusted. see here

I think that the adjustment should be:

ricSig <- ricSig * sqrt(1 - rho^2)

and not:

ricSig <- ricSig^2 * (1 - rho^2)

The adjustment is also done twice, in line 520 and in line 548

I think we should remove one of these.

Let me know what you think!

carrieholt commented 1 year ago

Yes, I agree. I don't think we used AR(1) version of the Ricker in our work with Cam (Fraser or Nass), so I don't think those lines were ever implemented. It looks like ricSig was mislabeled when it was supposed to be ricVar in that context (coding likely changed, and that line should have been updated). Yes, to removing one. If ricSig does not change among trials (which I don't think it does...), then can omit the second.

carrieholt commented 1 year ago

I can go ahead and do it, unless you're already in the code changing other things?

catarinawor commented 1 year ago

I just pushed the fix described above. I realized that the second instance of the code is only used when the parameter values are pulled from a posterior distribution and therefore change in every trial, as @carrieholt pointed out. So I left both instances.