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

Fix seasonal recruitment #284

Merged
merged 4 commits into from
Apr 13, 2022
Merged

Fix seasonal recruitment #284

merged 4 commits into from
Apr 13, 2022

Conversation

Rick-Methot-NOAA
Copy link
Collaborator

@Rick-Methot-NOAA Rick-Methot-NOAA commented Apr 5, 2022

What issue(s) does this PR address? Describe and add issue numbers, if applicable.

What tests have been done? Upload any model input files created for testing in a zip file, if possible.

compare output between tables for the anchovy assessment files

What tests/review still need to be done? Who can do it, and by when is it needed (ideally)?

another sharp set of eyes

Check which is true. This PR requires:

  • Changes in r4ss
  • Changes to SS3 manual
  • Changes to SSI (the SS3 GUI)
  • An entry in the stock synthesis change log (new features, bug reports)

Describe any changes in r4ss/SS3 manual/SSI/change log that are needed (if checked):

Additional information (optional):

SS_write_report.tpl Outdated Show resolved Hide resolved
@Rick-Methot-NOAA
Copy link
Collaborator Author

Rick-Methot-NOAA commented Apr 5, 2022

This second issue was detected while diagnosing the same set of input files that use empirical wtatage (wtatage==1) . The problem was that fecundity, as calculated from growth parameters, was erroneously overwriting the fec(g) array. This occurred when save_for_report was turned on. This problem showed up in the biology used for SPR/YPR profile differing from the biology used for benchmark. The fix was to add a conditional statement in get_mat_fec()
if(WTage_rd==1){
fec(g)=Wt_Age_t(t,-2,g);
}
else{ // make fecundity from biology

@Rick-Methot-NOAA
Copy link
Collaborator Author

r4ss problems probably due to some extra output lines that are not finalized.

@k-doering-NOAA
Copy link
Contributor

r4ss problems probably due to some extra output lines that are not finalized.

Yes, that makes sense. We should probably get those sorted out before merging into main, right?

Here is the error: https://github.com/nmfs-stock-synthesis/stock-synthesis/runs/5841949938?check_suite_focus=true#step:12:72

<simpleError in order(yielddat[["Depletion"]], decreasing = FALSE): argument 1 is not a vector>

@k-doering-NOAA
Copy link
Contributor

This second issue was detected while diagnosing the same set of input files. It is due to the non-modularity of the growth&biology parameters vs. the input of all biology from wtatage.ss The problem was that fecundity, as calculated from growth parameters, was erroneously overwriting the fec(g) array. Really should have been a separate issue with separate fix.

Thanks for explaining!

@Rick-Methot-NOAA
Copy link
Collaborator Author

with nearly every check passed, I will let this issue sit until I return to office on 4/11,

@Rick-Methot-NOAA Rick-Methot-NOAA merged commit 1486a3d into main Apr 13, 2022
@Rick-Methot-NOAA Rick-Methot-NOAA linked an issue Apr 13, 2022 that may be closed by this pull request
@k-doering-NOAA k-doering-NOAA deleted the fix_seasonal_recruitment branch April 15, 2022 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

errors in seasonal recruitment and empirical wtatage
2 participants