[Bug]: size frequency output in CompReport.sso has same method for all observations

[iantaylor-NOAA]

Describe the bug

Bug found by Yohei Tsukahara (while exploring updating the SS3 version for a Pacific Bluefin Tuna model) is causing incorrect values for the size frequency method in CompReport.sso. It's the same value for all observations. This causes an error when plotting the size frequencies in r4ss because it can't figure out which units to use for the observations !error with size units in generalized size comp plots: more than one unit value per method. This appears to just be a reporting issue and does not impact estimation.

The problem seems to have appeared between 3.30.19 and 3.30.20, perhaps as part of this pull request https://github.com/nmfs-ost/ss3-source-code/pull/357.

I think that the "k" at the end of this line should probably be "Sz_method": https://github.com/nmfs-ost/ss3-source-code/blob/f3fec9199296c44d912984245833ac4f7cad1b56/SS_write_report.tpl#L4273C1-L4273C1

To Reproduce

Run the attached files in 3.30.20 or later. simple_small_sizecomp.zip

Expected behavior

The "Ageerr" column in CompReport for all the SIZE observations at the end should be 1 or 2, not 3 as in the CompReport_3.30.20.sso file in the attached zip.

Screenshots

No response

Which OS are you seeing the problem on?

Windows

Which version of SS3 are you seeing the problem on?

3.30.20 and beyond

Additional Context

No response

[Rick-Methot-NOAA]

That seems a correct interpretation of the code error. Can you fix while I work on survey bias adjustment?

[iantaylor-NOAA]

Draft Pull Request #540 fixes the size frequency method in CompReport.sso, but while testing I saw that the size frequency section in data_boot_001.ss and data_expval.ss are identical to the input file (but were different in 3.30.19).

It looks like this commit 61d2cba7ea9ce1d640920ec242c54098f039443e, which included a variety of other changes, may have inadvertently led to the observed output getting written to all three data file types.

In particular this change looks incorrect: https://github.com/nmfs-ost/ss3-source-code/commit/61d2cba7ea9ce1d640920ec242c54098f039443e#diff-17a83800a032b2061d3ddd5c6da9d766d9796ecb81f2ebf1599b9808c206247eL775-R782 but the change includes moving from looping over size frequency observations to size frequency methods, so I'm assuming the fix is not as simple as swapping SzFreq_obs and SzFreq_exp.

I can work on this more next week but need to wrap up another project first. We can also merge the fix in #540 and create a separate issue and pull request for this one.

Lesson from all this: more tests related to generalized size frequency and/or bootstrap data would be helpful. The one test model with generalized size frequency data has the same scale and units for both size frequency methods: https://github.com/nmfs-ost/ss3-test-models/blob/main/models/two_morph_seas_areas/data.ss#L455. And I don't think we have any tests for expected values or bootstrap data.

[Rick-Methot-NOAA]

Thank you Ian for finding this issue. Let us finish #540, then create new issue as you suggest. I can take the lead on responding to the new issue as it seems I created the problem. A decision I made long ago in creating the *.new files was to have three big loops through the whole data file for obs, exp, and boot. Rather than one loop through the data file and conditional statements in each section for the 3 types of output.

[e-perl-NOAA]

I created a new issue for the size frequency output.

[iantaylor-NOAA]

Fixed by #540. New issue #541 opened to cover the bootstrapping.