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 kriging apportionment method and functions #215

Merged
merged 56 commits into from
Apr 12, 2024

Conversation

brandynlucca
Copy link
Collaborator

This includes:

  • Refactoring code to make the apportionment function easier to read
  • Continue translating the original Matlab code to complete the total biomass calculation (which still requires refactoring and formatting, but the code is otherwise present)
  • Begin adding unit tests for the kriging apportionment functions

Brandyn Lucca and others added 29 commits March 6, 2024 11:38
- This pertains to the kriging apportionment operation
- Also rearranges how `compute_summed_aged_proportions` is called
to calculate the aged proportions since the sex proportions can be calculated
in paralell with the total aged proportions in this step.
@leewujung
Copy link
Member

Just a note: for this one is fine, but for the next ones please do it from your fork.

Comment on lines +237 to +241
# ---- Drop unaged fish
# ==== !!! TODO: pending what FEAT says, weights associated with
# ==== missing ages should be added into the 'unaged' category.
# ==== This would further mean apportioning these into `weight_strata_aged_uanged`
specimen_data_filtered = specimen_data[ specimen_data.sex != 'unsexed' ].dropna( how = 'any' , subset = 'age' )
Copy link
Member

Choose a reason for hiding this comment

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

This does 2 things: select only sexed fish and dropped unaged fish, right?

Are there other place where unsexed fish from station 1 is used? If not, maybe we could also drop unsexed fish from station 1 when specimen_df is first read in. Or is it that there are too many unsexed fish so it is not a good idea to just drop them at data loader?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • Drop unaged fish in data loader
  • Adding caveat doc page related to the treatment of unsexed and removal of unaged fish
  • Which functions do the unsexed fish become relevant (e.g. length-weight regression)
  • Print out in console in data loader: "x unaged samples removed from Station 2 dataset" or something to that effect

Copy link
Member

@leewujung leewujung left a comment

Choose a reason for hiding this comment

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

Hey @brandynlucca : Thanks for the PR! I made various inline comments. Below are some overall comments:

Under the function apportion_kriged_biomass, the outputs weight_strata_aged_unaged and unaged_biomass appear unused after they are created. Since you went all the way to the end for the apportioning, this means that these variables can be removed, right?

For dropping unaged fish in station 1, how about dropping those fish when you read in specimen_df in the data loader? This way we don't have to worry about dropping those separately in each functions. It would be as if those fish samples never exist.

I guess the above may not be true for unsexed fish, but in compute_aged_weight_proportions the unsexed fish are also dropped (not selected, here). Should these be removed at the data loader too? Or because the number is larger and these samples are used in some other processing assuming the unsexed fish are assumed to follow the same male:female proportion, we should not drop them at data loader?

Lastly, there are two types of function names compute_* and calculate_* under biology.py -- is that intentional? Seems that compute_* are functions that are called by calculate_*?

@brandynlucca brandynlucca merged commit 9b9f1b4 into main Apr 12, 2024
2 checks passed
@brandynlucca brandynlucca deleted the brandynlucca_WIP_kriging_biomass_apportionment branch April 12, 2024 23:22
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.

2 participants