fix for tag mirror #268; partial fix for retro-superperiod #265

[Rick-Methot-NOAA]

What issue(s) does this PR address? Describe and add issue numbers, if applicable.

• [mirroring of dispersion parameters for tags not working #268 and retro and superperiods #265 ] Link issue on pull request

What tests have been done? Upload any model input files created for testing in a zip file, if possible.

retro fix is bandaid only for lencomp superperiod, but works
fix for tag mirroring needs testing

What tests/review still need to be done? Who can do it, and by when is it needed (ideally)?

fix for tag mirroring needs testing

Check which is true. This PR requires:

• Changes in r4ss
• Changes to SS3 manual
• Changes to SSI (the SS3 GUI)
• An entry in the stock synthesis change log (new features, bug reports)
  none. these are minor fixes

Describe any changes in r4ss/SS3 manual/SSI/change log that are needed (if checked):

[k-doering-NOAA]

Hi Rick, thanks for getting this up so quickly! How do you advise I test that tag mirroring fix?

This model already includes mirrored tagging overdispersion parameters; however, I find no difference in likelihood between this PR and 3.30.18 when I run the model. In addition, I can see the mirroring (-1000) propogated into the control.ss_new file with 3.30.18.

Could you tell me what I should be looking at to see if the fix worked? I wasn't sure...

[k-doering-NOAA]

Next time, it would be nice to break the changes addressing 2 issues into separate commits (although I think 1 PR is fine!)

[k-doering-NOAA]

Actually, I figured it out how to test, I think - the case I was testing happened to be a case not eaffected by the code.

I changed up the model (mirrored 2 fleets instead of 1, and now get a .18 model that runs, and the pull request version with this error:

tau <=1 in log_negbinomial_density

Not sure if this is because the values I changed the tag dispersion parameters to don't make sense, or if there is something else going on?

Zipped model files

[iantaylor-NOAA]

@k-doering-NOAA, I think your modified model and the original in test-models should both be setting all the tag overdispersion parameters fixed at 1.001, with the only difference being which ones are mirroring which others. Presuming that your test model works fine with Rick's fix then I think it's a good test.

[Rick-Methot-NOAA]

I can check tomorrow. Change for tags is in objfun. Don’t see how old code could actually do mirror.

…

Sent from my iPhone On Mar 8, 2022, at 1:40 PM, Kathryn Doering ***@***.***> wrote:  Hi Rick, thanks for getting this up so quickly! How do you advise I test that tag mirroring fix? This model <https://github.com/nmfs-stock-synthesis/test-models/blob/main/models/tagging_mirrored_sel/control.ss#L578> already includes mirrored tagging overdispersion parameters; however, I find no difference in likelihood between this PR and 3.30.18 when I run the model. In addition, I can see the mirroring (-1000) propogated into the control.ss_new file with 3.30.18. Could you tell me what I should be looking at to see if the fix worked? I wasn't sure... — Reply to this email directly, view it on GitHub <#270 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABPV4IDVGX3R5DAPQDKIUYTU67CNBANCNFSM5QHM7USA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[yukio-takeuchi]

I would like to raise one more related change on the tag parameter mirroring
In lines 20-21 of SS_tagrecap.tpl, codes for tag reporting rate mirroring are

      if (TG_parm_PH(j)>-1000.)	{k=j;} else {k=-1000-TG_parm_PH(j);}
      	TG_report(f)=mfexp(TG_parm(k))/(1.+mfexp(TG_parm(k)));

For the case if TG_parm_PH(j) <-1000 (else part),
I suspect, addition of 3N_TG as an offset is needed similar to what has been done for TAG chronic loss parameter in line 67
Otherwise if TG_parm_PH(j) < -1000 (eg -1000-x) and j=3N_TG +f, k becomes f so TG_parm(x) is used to calculate reporting rate instead of using TG_parms(3N_TG+ x),
Also I suspect similar addition of 3N_TG is needed for the same part fo TG_rep_decay

[Rick-Methot-NOAA]

Thank you very much Yukio for taking such a close look at the code in a section that is not used by any in our local assessment team, so weakly tested. We will make this change also.

[Rick-Methot-NOAA]

tau <=1 in log_negbinomial_density
is an internal ADMB error message. So, the parameter value must be out of range. I am working with this example now to create a testbed to improve the tag mirroring code.

[yukio-takeuchi]

tau <=1 in log_negbinomial_density is an internal ADMB error message. So, the parameter value must be out of range. I am working with this example now to create a testbed to improve the tag mirroring code.

The functions which calculate log ofe negative binomial density in ADMB are

 double log_density_negbinomial(double x,double mu,double tau)
  {
    if (tau-1.0<0.0)
    {
      cerr << "tau <=1 in log_negbinomial_density " << endl;
      ad_exit(1);
    }
    double r=mu/(1.e-120+(tau-1.0));
    return gammln(x+r)-gammln(r)-gammln(x+1)
      +r*log(r)+x*log(mu)-(r+x)*log(r+mu);
  }

in linad99/cnegbin.cpp and its dvariable version in vnegbin.cpp
Current SS seems to substitute overdispersion to tau.
overdispersion may always need slightly larger than 1.

[Rick-Methot-NOAA]

Thanks. This error is happening because inappropriate value was being mirrored, so gets fixed by the coding error you found.
I will keep name as overdisp because tau is used already in context of beta priors

[yukio-takeuchi]

I agree to continue to use overdisp in stead of tau.
I also propose to mention overdisp is named as tau internally in ADMB in future user manual, so that anyone can avoid their confusion when they see the error message.
Also it might be useful to note that with overdisp variance of NB can be written as overdisp * “expected value” in user manual as well.

[Rick-Methot-NOAA]

now with new warnings:
8 overdispersion par_min is <1.0 for TG= 1; value = 0.99; changed to 1.001 for run
9 overdispersion parameter is <1.0 for TG= 1; value = 0.995; changed to 1.001 for run

[yukio-takeuchi]

run

Great.

Many thanks for your work