Skip to content

first commit of discard body wt output to report 32 - #790

Merged
Rick-Methot-NOAA merged 5 commits into
mainfrom
output_disc_bodywt
Nov 10, 2026
Merged

Rick-Methot-NOAA merged 5 commits into
mainfrom
output_disc_bodywt

Conversation

@Rick-Methot-NOAA

@Rick-Methot-NOAA Rick-Methot-NOAA commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

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

What tests have been done?

Where are the relevant files?

using simple with discard test file. in output, look for runs in report 32 with "disc_bodywt" and "ret_bodywt"

What tests/review still need to be done?

Is there an input change for users to Stock Synthesis?

Additional information (optional).

@e-perl-NOAA

Copy link
Copy Markdown
Collaborator

There was something wonky in the code to run the tests for the tests models and bootstrap, I just fixed these so hopefully those tests will pass now

@e-perl-NOAA

Copy link
Copy Markdown
Collaborator

@Rick-Methot-NOAA, I'm not sure if this concern is real or not but when copilot inspected the PR it flagged the following:

The calculation is only added to the size-selectivity in SS_selex.tpl (around lines 2029–2035), but the report writes these outputs whenever Do_Retain(f) > 0. Age-based retention is handled separately, so fleets using age-based retention can get un-computed or stale values in the new output. Either calculate the values for age-based retention too, or limit the report output to fleets with size-based retention.

@Rick-Methot-NOAA

Copy link
Copy Markdown
Collaborator Author

Thanks for the reminder CoPilot. You are correct that I need to omit age-retention from the reporting of this quantity. Glad that you did not see any flaw in the calculation itself.

@e-perl-NOAA

Copy link
Copy Markdown
Collaborator

I had copilot review things again and it came up with the following:

For SS_write_report.tpl lines 2991–2993:

Could we handle WTage_rd == 1 here? Length-based retention is allowed with empirical weight-at-age, but in that mode Make_FishSelex takes the empirical-weight branch and never populates the new disc_age slots. This code still emits them, so disc_bodywt and ret_bodywt won’t contain calculated partition weights. Could we populate those slots for this mode or skip emitting them?

@Rick-Methot-NOAA

Copy link
Copy Markdown
Collaborator Author

@e-perl-NOAA Length-based retention should not be allowed with WTage_rd == 1. Length-based retention requires a length-distribution calculated from the age-length key. None of that is meaningful when WTage_rd == 1, even though it is calculated. If I could roll back the clock, I would have enforced elimination of all growth and ALK quantities when WTage_rd == 1. I'll add a trap for WTage_rd == 1 here to prevent the output.

@e-perl-NOAA

Copy link
Copy Markdown
Collaborator

@Rick-Methot-NOAA, yeah of course, I was just communicating what copilot saw and had questions about when reviewing the PR again

@e-perl-NOAA e-perl-NOAA left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems good to me after having copilot review it 2x, idk if you want to wait for @iantaylor-NOAA to review it before merging.

@Rick-Methot-NOAA

Copy link
Copy Markdown
Collaborator Author

There will need to be an r4ss augmentation in order to read and display this new output.

@iantaylor-NOAA iantaylor-NOAA left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes all look good to me. There are no modifications needed in r4ss to read the output as the new rows are part of the standard format of the AGE_SELEX table.

I also testing converting the SS3 output to the standard format used by stockplotr using first r4ss::SS_output() and then passing that list to stockplotr::convert_output(). The new rows are available via a command like
converted_output |> dplyr::filter(module_name == "ageselex" & label == "disc_bodywt").

There are currently no r4ss functions that work with either the mean body weight at age output, so I think any new plots of this data type could be added to stockplotr instead of r4ss.

@iantaylor-NOAA iantaylor-NOAA left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to click "approve" in my previous review so it came through as a comment. Posting again to be able to "approve"

@Rick-Methot-NOAA
Rick-Methot-NOAA merged commit 9c77dd2 into main Nov 10, 2026
21 checks passed
@Rick-Methot-NOAA
Rick-Methot-NOAA deleted the output_disc_bodywt branch November 10, 2026 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Output body weight-at-age of retained and discarded fish when retention function is used

3 participants