search

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
38 stars 16 forks source link

first commit for joinsteep=50 #623

Closed Rick-Methot-NOAA closed 1 week ago

Rick-Methot-NOAA commented 1 month ago

Concisely describe what has been changed/addressed in the pull request.

Replace all the individual join function constants with the constant termed joinsteep and with a value of 50. Values previously ranged from 10 to 1000.

What tests have been done?

see comparisons in the issue

Where are the relevant files?

What tests/review still need to be done?

Is there an input change for users to Stock Synthesis?

Additional information (optional).

Rick-Methot-NOAA commented 1 month ago

perhaps I should revert back to the previous big values (1000) and small values (10) to see which had the biggest impact.

iantaylor-NOAA commented 1 month ago

I manually ran the run-ss3-with-est github action to see the impact of the joiner change on all models. (I'm not sure why this action isn't being run every time, perhaps we turned them off to save time?).

The files in the artifact from the action are in this zip: https://github.com/nmfs-ost/ss3-source-code/actions/runs/11018401010/artifacts/1972890354 including a table of the changes to a set of quantities of interest from all the test models.

The one model that failed the test is the "three_area_nomove" model where the ForeCatch_2010_se increased from 1.28173e+03 to 2.44708e+03.

For future reference, comparison is done by this R function https://github.com/nmfs-ost/ss3-test-models/blob/main/.github/r_scripts/compare.R where for most quantities a change of greater than 1% fails the test but there are a few exceptions, like max gradient, where an absolute change of more than 0.001 causes it to fail.

Rick-Methot-NOAA commented 1 month ago

I agree with all you are saying and like your logic about the categories. I remain uncomfortable that so many values end up with small changes based on ill-informed differences 20 vs 30 vs 40. I changed the 1000 and 10 instances back to that value, but still find differences. Now I am curious which one is making the biggest impact, but don't want to go through the tedious exercise of changing them one at a time. Shelving for now; will do fix for #622 separately.