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

update(SWFSC-sardine.qmd): Update functionality for 0.3.0.0 #53

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

alexjensen-NOAA
Copy link
Contributor

What is the feature?

  • Updated SWFSC-sardine.qmd to utilize new wrapper functions, input WAA data into FIMSFrame, and pull correct outputs from FIMS model fitting, all associated with FIMS v0.3.0.0, in addition to generating new comparison figures with the new model run

How have you implemented the solution?

  • Revised, added, and removed code in SWFSC-sardine.qmd

Does the PR impact any other area of the project, maybe another repo?

  • No

Copy link
Contributor

@kellijohnson-NOAA kellijohnson-NOAA left a comment

Choose a reason for hiding this comment

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

THANK YOU! My only question is, should weight at age be wtatage$value/1000?

@@ -105,6 +103,7 @@ fimscatch <- tibble(type = "landings", name = "fleet1",
age = NA, datestart = paste0(catch$year, "-01-01"),
dateend = paste0(catch$year, "-12-31"), value = catch$catch,
unit = "mt", uncertainty = 0.05)
fimscatch$value <- fimscatch$value
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure what this line is doing?

Copy link
Contributor

Choose a reason for hiding this comment

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

can delete I think

fimswaa <- tibble(type = "weight-at-age", name = wtatage$fleet,
age = wtatage$age, datestart = paste0(wtatage$year, "-01-01"),
dateend = paste0(wtatage$year, "-12-31"),
value = wtatage$value, unit = "kg", uncertainty = NA)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should value be wtatage$value/1000?

Copy link
Contributor

Choose a reason for hiding this comment

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

THe units for wtatage$value are already kg, so I don't think it should be divided by 1000

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It depends on what FIMS is expecting to see for WAA units. From the Seaside Chat on Thursday, it sounded as if WAA should be input to FIMS using mt, not kg, which would require the '/1000' adjustment to the conventional input to SS3.

With this adjustment, the summary biomass and numbers at age match for FIMS and SS3 (assuming that FIMS reports numbers at age in terms of fish and SS3 reports numbers of age in terms of thousands of fish).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only noticed the need for the WAA adjustment after the initial pull request. I can add in the '/1000' adjustment, remove the redundant line (above), and submit a new pull request, if that's the easiest way to move forward

Copy link
Contributor

Choose a reason for hiding this comment

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

No need for a new PR, just make the necessary changes, add the file(s), commit, and push to this branch. Then the PR will be updated.

@Rick-Methot-NOAA
Copy link

Alex - the SS3 convention is that body wt is in kg, catch and population totals are in mt, and numbers are in thousands.

- Updated SWFSC-sardine.qmd to utilize new wrapper functions
- input WAA data into FIMSFrame
- Adjust WAA inputs to FIMS by dividing by 1000, convert kg (SS3) to mt (FIMS)
- pull correct outputs from FIMS model fitting
- Generated new comparison figures with new model run
- Adjust reported recruitment from FIMS to match SS3 units (divide by 1000)
- uses packagename:: syntax for almost everything but FIMS
- converted to base pipe
@kellijohnson-NOAA kellijohnson-NOAA merged commit acd7551 into main Jan 22, 2025
1 check failed
@kellijohnson-NOAA kellijohnson-NOAA deleted the update_sardine branch January 22, 2025 16:17
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.

4 participants