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.

Sign up for GitHub

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

Jump to bottom

[Bug]: proportion female at age #638

Open
5 tasks done
KyleShertzer-NOAA opened this issue Jun 29, 2024 · 5 comments
Open
5 tasks done

[Bug]: proportion female at age #638

KyleShertzer-NOAA opened this issue Jun 29, 2024 · 5 comments
Assignees
@kellijohnson-NOAA
Labels
kind: bug Something isn't working kind: documentation Improvements or additions to documentation kind: refactor Restructure code to improve the implementation of FIMS P3 low priority task theme: code cleanup theme: R interface
Milestone
2

Comments

@KyleShertzer-NOAA
Copy link
Contributor

KyleShertzer-NOAA commented Jun 29, 2024 •
edited by kellijohnson-NOAA
Loading

Describe the bug

A vector of sex ratio at age appears to be accepted as FIMS input, using "population$proportion_female <- vector"
However, this capability is not currently implemented, with sex ratio hard coded at 0.5.

To Reproduce

No response

Screenshots

No response

Which OS are you seeing the problem on?

Windows

Which browser are you seeing the problem on?

Other

Which version of FIMS are you seeing the problem on?

FIMS_0.2.0.0

Additional Context

For the southeast scamp example, I want to specify the sex ratio at age as 100% female. This is a work-around to mimic how BAM handles SSB of a protogynous hermaphrodite, as total (male + female) mature biomass. Current FIMS assumes sex ratio is 50:50, which means that in the scamp example, FIMS-predicted SSB is half that of BAM-predicted SSB.

Tasks
Preview Give feedback
@KyleShertzer-NOAA KyleShertzer-NOAA added kind: bug Something isn't working status: triage_needed This is not approved for this milestone, do not work on it yet labels Jun 29, 2024
@kellijohnson-NOAA
Copy link
Contributor

This older PR is relevant. @Bai-Li-NOAA will work with the "fixer" of this bug to help design tests.

Thoughts for fixing ... "prepare" function in population where a lot of values are set and it is possible that it is being reset in the prepare function.

@kellijohnson-NOAA kellijohnson-NOAA added this to the 2 milestone Jul 9, 2024
@kellijohnson-NOAA
Copy link
Contributor

Thank you @KyleShertzer-NOAA for bringing this up. During NOAA-FIMS/seasid-chats#7 we decided to remove the capability of the user to set this because it is not currently doing anything and then later in the future we can revisit what proportion_female should be doing. There is now a task list at the top of this issue for how to fix it.

kellijohnson-NOAA added a commit that referenced this issue Jul 25, 2024
@kellijohnson-NOAA
fix(proportion_female): Remove user interface
24a5457 
Users thought that they could set proportion_female but it was hard-
coded to 0.5 on the back end. Now, it is back to just being a single
value that is hard coded to 0.5. Tests have also changed back to 0.5.
This partially reverts PR #543. And, was talked about in
NOAA-FIMS/seaside-chats#7 and partially addresses #638.
kellijohnson-NOAA added a commit that referenced this issue Jul 25, 2024
@kellijohnson-NOAA
fix(proportion_female): Remove user interface
fb13fe5 
Users thought that they could set proportion_female but it was hard-
coded to 0.5 on the back end. Now, it is back to just being a single
value that is hard coded to 0.5. Tests have also changed back to 0.5.
This partially reverts PR #543. And, was talked about in
NOAA-FIMS/seaside-chats#7 and partially addresses #638.
kellijohnson-NOAA added a commit that referenced this issue Jul 25, 2024
@kellijohnson-NOAA
fix(proportion_female): Remove user interface
0a82bcb 
Users thought that they could set proportion_female but it was hard-
coded to 0.5 on the back end. Now, it is back to just being a single
value that is hard coded to 0.5. Tests have also changed back to 0.5.
This partially reverts PR #543. And, was talked about in
NOAA-FIMS/seaside-chats#7 and partially addresses #638.
kellijohnson-NOAA added a commit that referenced this issue Jul 25, 2024
@kellijohnson-NOAA
fix(proportion_female): Remove user interface
8995cd6 
Users thought that they could set proportion_female but it was hard-
coded to 0.5 on the back end. Now, it is back to just being a single
value that is hard coded to 0.5. Tests have also changed back to 0.5.
This partially reverts PR #543. And, was talked about in
NOAA-FIMS/seaside-chats#7 and partially addresses #638.
kellijohnson-NOAA added a commit that referenced this issue Jul 25, 2024
@kellijohnson-NOAA
fix(proportion_female): Remove user interface
577a68b 
Users thought that they could set proportion_female but it was hard-
coded to 0.5 on the back end. Now, it is back to just being a single
value that is hard coded to 0.5. Tests have also changed back to 0.5.
This partially reverts PR #543. And, was talked about in
NOAA-FIMS/seaside-chats#7 and partially addresses #638.
@kellijohnson-NOAA kellijohnson-NOAA self-assigned this Jul 25, 2024
kellijohnson-NOAA added a commit to NOAA-FIMS/case-studies that referenced this issue Jul 26, 2024
@kellijohnson-NOAA
refactor(proportion_female): Remove comments
e05708f 
proportion_female was commented out in all of the case studies because
setting it was not doing anything to the model. In M2 we will remove the
ability to set this parameter vector because it was not doing anything.
In the future we will add the feature back. This change does nothing to
the results.

Simplified a comment in the scamp case study to say that it is fixed at
0.5 instead of you have the ability to estimate it.

Part of NOAA-FIMS/FIMS#638 and NOAA-FIMS/seaside-chats#7
@kellijohnson-NOAA kellijohnson-NOAA mentioned this issue Jul 26, 2024
Merged
kellijohnson-NOAA added a commit to NOAA-FIMS/case-studies that referenced this issue Jul 26, 2024
@kellijohnson-NOAA
refactor(proportion_female): Remove comments
3e8f058 
proportion_female was commented out in all of the case studies because
setting it was not doing anything to the model. In M2 we will remove the
ability to set this parameter vector because it was not doing anything.
In the future we will add the feature back. This change does nothing to
the results.

Simplified a comment in the scamp case study to say that it is fixed at
0.5 instead of you have the ability to estimate it.

Part of NOAA-FIMS/FIMS#638 and NOAA-FIMS/seaside-chats#7
@kellijohnson-NOAA kellijohnson-NOAA added kind: documentation Improvements or additions to documentation kind: refactor Restructure code to improve the implementation of FIMS P3 low priority task theme: R interface theme: code cleanup and removed status: triage_needed This is not approved for this milestone, do not work on it yet labels Jul 26, 2024
kellijohnson-NOAA added a commit that referenced this issue Aug 31, 2024
@kellijohnson-NOAA
fix(proportion_female): Removes faulty est input
6eb54ec 
proportion_female appeared to be estimable and/or specified by the user
but it was still hard coded to 0.5 for every age. This commit reverts
back part of PR #543 where the hard-coded value (not by age) was made
0.5 by age with a user interface.

Part of #638
kellijohnson-NOAA added a commit that referenced this issue Sep 1, 2024
@kellijohnson-NOAA
fix(proportion_female): Removes faulty est input
86a2a1a 
proportion_female appeared to be estimable and/or specified by the user
but it was still hard coded to 0.5 for every age. This commit reverts
back part of PR #543 where the hard-coded value (not by age) was made
0.5 by age with a user interface.

Part of #638
kellijohnson-NOAA added a commit that referenced this issue Dec 30, 2024
@kellijohnson-NOAA
fix(proportion_female): Removes faulty est input
97ad296 
proportion_female appeared to be estimable and/or specified by the user
but it was still hard coded to 0.5 for every age. This commit reverts
back part of PR #543 where the hard-coded value (not by age) was made
0.5 by age with a user interface.

Part of #638
@kellijohnson-NOAA
Copy link
Contributor

@KyleShertzer-NOAA I started a design document for this. I am wondering if you would be willing to populate it with some initial text that we can have later reviewed by the FIMS Implementation Team? If you do not have time, would you mind suggesting someone else that you think would be good to do the initial drafting. Thank you. The deadline for this initial draft will be the first week of March 2025.

@kellijohnson-NOAA
Copy link
Contributor

The code to fix the bug of users thinking that this was estimable was fixed in 2338d23.

@KyleShertzer-NOAA
Copy link
Contributor Author

@kellijohnson-NOAA Yes, I will draft some initial text for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kellijohnson-NOAA kellijohnson-NOAA

Labels
kind: bug Something isn't working kind: documentation Improvements or additions to documentation kind: refactor Restructure code to improve the implementation of FIMS P3 low priority task theme: code cleanup theme: R interface
Projects
M2 R Interface
Status: No status
Milestone
2
Development

No branches or pull requests
2 participants
@kellijohnson-NOAA @KyleShertzer-NOAA