nmfs-ost / ss3-source-code

The source code for Stock Synthesis (SS3).
https://nmfs-ost.github.io/ss3-website/
Creative Commons Zero v1.0 Universal
37 stars 16 forks source link

retro and superperiods #265

Open iantaylor-NOAA opened 2 years ago

iantaylor-NOAA commented 2 years ago

Vlab forums message from Alessandro Orio indicates issue with superperiods in retrospectives https://vlab.noaa.gov/web/stock-synthesis/public-forums/-/message_boards/view_message/20972432

Rick, I suspect that this one will need you to tackle as I've never looked under the hood at the super-period logic before. Please let us know if there's ways that others of us can help.

Rick-Methot-NOAA commented 2 years ago

I will look, but fear a generalized fix may exceed a reasonable benefit/effort.

iantaylor-NOAA commented 2 years ago

Thinking about this more, it occurs to me that it might not even make sense to attempt a generalized fix, regardless of the time commitment, because there's no way for the software to understand the nuances of the super-period, in terms of what data would be grouped as a super-period if fewer years of data were available.

Requiring the rare user of super-periods to manually modify 5 separate data files for a 5-year retro doesn't seem like a terrible burden considering that by the point the retros are being conducted, the data file is typically very stable.

Maybe it would be best to just augment warning such that if retro year != 0 some additional message is displayed about the need for manual edits to the super-periods.

Rick-Methot-NOAA commented 2 years ago

I agree. Much higher priority to make retro play nice with time-varying parameters.

Rick-Methot-NOAA commented 2 years ago

new VLAB forum post said that superperiods and retro worked in 3.24, so I will investigate this issue for 3.30.19. Example files are attached to the VLAB post.

Rick-Methot-NOAA commented 2 years ago

The model stops because of a new fatal warning that was added to detect erroneous situations with multiple real observations in a superperiod.

With the retroyr, there are zero real obs in the terminal super-years.  However the warning condition was set to <>1, rather than >1, so the model stops.

So, this should be a rather easy fix in the preliminary calcs section of the code.  Only hitch is that all the obs now have the "ignore" flag, so prelim does not have needed info to assign relative sample weight. SS_data does this: header_l(f,j,3)=lendatai; if(y>retro_yr) header_l(f,j,3)=-f;

then SS_prelim misinterprets (ignore that i,j different here than in data): if(header_l(f,i,3)<0) // so one of the obs to be combined {temp+=nsamp_l(f,i);}

So, the sign of lendatai could be made available to ss_prelim but that would require adding another vector of info. Alternatively: in ss_dataread, only assign the -f flag if the obs is not part of a superyear, then in ss_prelim, assign the flag inside the loop creating obs weights. Workable, but convoluted logic.

Aside from this immediate issue, there is the general issue of a retro year crossing boundary of a super-year and causing havoc.

Rick-Methot-NOAA commented 2 years ago

Let's leave this issue open for potential future action. A partial fix was implemented for lencomp superperiods such that a fatal exit would not be triggered, but any superperiod than spans retroyr will not be implemented as intended (weighting of obs within the superperiod will be incorrect) and the user will get no warning.