Skip to content
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

clean up warnings in test-models #51

Open
iantaylor-NOAA opened this issue Oct 31, 2023 · 5 comments
Open

clean up warnings in test-models #51

iantaylor-NOAA opened this issue Oct 31, 2023 · 5 comments
Assignees

Comments

@iantaylor-NOAA
Copy link
Contributor

iantaylor-NOAA commented Oct 31, 2023

The models in the test-models repo have some warnings that didn't exist (or related to model options that didn't exist) when the models were created. In most cases, the models should be changed to make the warnings go away. This will make it easier to see when new warnings appear in the future. In a few cases, the warnings themselves may need revision in the SS3 code.

@e-gugliotti-NOAA and @iantaylor-NOAA went through some of the warnings files and came up with the following ideas for how they can be dealt with. This list is incomplete and will be extended in the near future. Anyone may feel free to modify this list if you have additional input.

Warnings that should lead to changes in the model files

  • "param init value is less than parameter min" some of these are fixed parameters (e.g. maturity 50%) so the bounds should be changed, not necessarily the init value.
  • fishery_timing as 1 not -1: change to -1 (this option wasn't available when the model was created
  • "Suggestion: This model has just one settlement event. Changing to recr_dist_method 4 and removing the recruitment distribution parameters at the end of the MG parms section (below growth parameters) will produce identical results and simplify the model." -- good to change, but requires deleting the recruitment distribution parameters as noted in the warning. These are at the bottom of the MGParm section (e.g. 3 parameters starting with "RecrDist_GP_1")
  • priors not included -- change starter value setting for "Include prior_like for non-estimated parameters" from 0 to 1
  • warning about gradient larger than convergence criterion -- If the value for "final convergence criteria" in starter.ss is tiny (e.g. 1e-05 or 1e-07), maybe make it slightly bigger (e.g. 1e-04).
  • size at peak < minimum data bin is illogical -- Change lower bound of selectivity to be equal to the first data bin (as reported in the warning)
  • "Information: A revised protocol for the Fcast_yr specification is available and recommended." (found in multiple models) we may want to switch to the revised protocol in the future, which would be easier if we wrote a simple R script to do the conversion.

Warnings that maybe need to be revised in SS3

  • "Block ends in 2030 after retroyr+1: 2019" (found in BigSkate_2019) -- not sure best solution here since end year of block is equal to end year of forecast (2030)
  • Max harvest rate for F method 2, 3, or 4 should be greater than 1 (found in "growth_timevary"). The F method is set to 1, not 2-4 so maybe this warning could be skipped?
  • "Minimum pop size bin is > L at Amin" -- maybe warning should not appear for empirical weight at age models?
  • "2D_AR selectivity deviations option is selected!" (found in Hake_2019_semi-parameteric_selex) -- not sure what value this warning has since the user has hopefully selected the option on purpose. If the goal is debugging accidental use of this option, it might be better to put notes into the echoinput to help with debugging.
  • "sd_ratio_read is < 0 , so expecting sd parameter after movement params" -- can we avoid printing this warning when the required sd_ratio parameter is present in control file?
  • [added 2 Nov]: * "1st iteration warning: ssb(endyr)/ssb(styr)= 5.31255e-05; suggest start with larger R0 to get near 0.4; or use depletion fleet option" Changing "depletion fleet" to "depletion survey" would make it easier to find the appropriate section of the user manual.

Warnings where we're not yet sure what to do

  • "Forecast F capped by max possible F from control file4" (found in KelpGreenling2015) -- not sure why this is occurring or what to do about it
  • Response: this is a real warning that indicates the selected forecast F approach is using a F level that is above above the specified upper limit. I will look at this model and comment further.
  • "At least one block pattern ends in endyr" (found in multiple models) -- not sure what to do here
  • Response: The idea here was to inform user that a time-varying parameter may be reverting to base for forecast. I'll try to make the logic better so it detects an actual problem, not just potential problem.
  • [added 1 Nov]: "Adjusted selparm out of base parm bounds" (found in Sablefish2015). Warning is useful, but it's not clear how best to change the model to make it go away--widening bounds on base parameter may impact model results (but probably still the first step to try). Would also be good to not repeat for multiple years when the time-varying is the same for all years within a block.
  • [added 1 Nov]: "Information: Forecast devs will be applied to mean base recruitment over range of historical years in forecast.ss" (found in both vermillion_snapper models)

Warnings that should probably stay

  • [added 1 Nov]: "Fmsy/mey is <1/3 of Fspr are you sure? check for convergence" (found in SpinyDogfish2011) I think this is a result of a known mismatch between biology and stock-recruitment assumptions and the SPR target set by management. Warning is useful and should stay.
  • [added 1 Nov]: "Reminder: Number of lamdas !=0.0 and !=1.0: 1" (found in two_morph_seas_areas, which has lambda = 2 for recdev likelihood). I think it's OK to have the lambda in the model and the useful reminder remain in the warnings file.
@Rick-Methot-NOAA
Copy link
Collaborator

Rick-Methot-NOAA commented Oct 31, 2023

This is a very useful starting list. Thanks for creating it. I will go through them and make suggestions. We should do the same for the examples we release to users. They should be warning free.

Regarding block pattern ending in endyr. This check is happening in the block pattern creation code, not in the timevary parm code which is where it will need to be if it is to be made more useful. I think this warning is still useful and users can ignore after checking it once.

@e-perl-NOAA
Copy link
Collaborator

@iantaylor-NOAA I saw a new note in the tagging_mirrored_sel model that is "N parameters that are on or within 1% of min-max bound: 2; check results, variance may be suspect" - is this a note that we should keep or should we try to change the model in some way?

@iantaylor-NOAA
Copy link
Contributor Author

I think we should leave it for now and explore changes to the model at some point in the future. I'm happy to take a look but probably don't have time this week.

@Rick-Methot-NOAA
Copy link
Collaborator

agreed. The parameter bound msgs would be hard to eliminate

@e-perl-NOAA
Copy link
Collaborator

Okay, no worries, just curious.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants