Skip to content

add warning for Lmin < len_bin(1)#698

Merged
Rick-Methot-NOAA merged 5 commits intomainfrom
682-feature-negative-length-leads-to-bad-fecundity-calculation
Aug 12, 2025
Merged

add warning for Lmin < len_bin(1)#698
Rick-Methot-NOAA merged 5 commits intomainfrom
682-feature-negative-length-leads-to-bad-fecundity-calculation

Conversation

@Rick-Methot-NOAA
Copy link
Copy Markdown
Collaborator

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

everything is in the issue and title.

What tests have been done?

Where are the relevant files?

<-- - [x] Test files are in the issue. -->

What tests/review still need to be done?

None

Is there an input change for users to Stock Synthesis?

<-- - [x] No, there was no input change. -->

Additional information (optional).

@Rick-Methot-NOAA
Copy link
Copy Markdown
Collaborator Author

Rick-Methot-NOAA commented Aug 4, 2025

at least one model is failing because the lower bound for Lmin is set out of bounds:
-10 45 21.6591 36 10 6 2 0 0 0 0 0 0 0 # L_at_Amin_Fem_GP_1

I prefer to keep the fatal warning, but I could revise to allow the lower bound to be any value if the parameter is not estimated. This would still test the initial value, but would skip the lower bound test if not estimated.

The blow-up in the calculations is in the code for the age-length key

@e-perl-NOAA
Copy link
Copy Markdown
Collaborator

@Rick-Methot-NOAA 10 of our test models are throwing this fatal error. I would suggest that we change these test models so that it doesn't throw the fatal error.

@e-perl-NOAA
Copy link
Copy Markdown
Collaborator

It looks like the run-with-est is (mostly) failing because the test models haven't been updated to the pre-release. I'm working on doing this, I'm just resolving an issue with the test models first. There are a couple of test model values that are different from when I run-with-est on main that we should double check once I get everything updated.

@e-perl-NOAA
Copy link
Copy Markdown
Collaborator

@Rick-Methot-NOAA I don't know why run-with-est isn't failing but run-no-est is, that is something that I will try to sort out. Regardless, there are some changes in the artifacts from the run-no-est that you should review.

@Rick-Methot-NOAA
Copy link
Copy Markdown
Collaborator Author

Did you check to see if the starter.ss is set to read from the par file? I just made that mistake in my initial local test.

@e-perl-NOAA
Copy link
Copy Markdown
Collaborator

e-perl-NOAA commented Aug 5, 2025

Ah, yes that is why there is a difference. The run with no est uses the par file and the run with est does not use the par file

@e-perl-NOAA
Copy link
Copy Markdown
Collaborator

I am going to do some local testing of this tomorrow morning though just to get a better idea of what is actually going on and see if I can spot anywhere that there may be an issue in the actions and/or the R code the actions use in the test models repo

@e-perl-NOAA
Copy link
Copy Markdown
Collaborator

@Rick-Methot-NOAA I have this passing all checks now 🎉

@Rick-Methot-NOAA Rick-Methot-NOAA merged commit c5259b3 into main Aug 12, 2025
9 of 11 checks passed
@Rick-Methot-NOAA Rick-Methot-NOAA deleted the 682-feature-negative-length-leads-to-bad-fecundity-calculation branch August 12, 2025 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Add warning to avoid use of negative length for the Lmin parameter

2 participants