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

[Bug]: FIMS runs regardless of the length of the log_obs_error and log_Fmort inputs #622

Open
Bai-Li-NOAA opened this issue Jun 6, 2024 · 2 comments
Assignees
Labels
kind: bug Something isn't working P1 high priority task
Milestone

Comments

@Bai-Li-NOAA
Copy link
Contributor

Describe the bug

FIMS runs even if the user provides only one value for log_obs_error when its length is supposed to be >1. FIMS extends the input with 0s when the input size is shorter than the number of years, and truncates extra values when the input is accidently longer than expected.

To Reproduce

See issue raised in the case-studies repo and a test demonstrating the behavior. In the test, FIMS runs when the length of fleet log_obs_error input is 1 or 100, although the expected length is 30.

Screenshots

No response

Which OS are you seeing the problem on?

Windows

Which browser are you seeing the problem on?

No response

Which version of FIMS are you seeing the problem on?

No response

Additional Context

Suggestions from @msupernaw: It looks like the problem is here, which is called after here. Initially, log_observed_error is resized to the short length in add_to_fims_tmb, and then resized again by Fleet::Initialize when Information::CreateModel. A warning message, or error would be appropriate if the vector is the incorrect length. This highlights the reason we need improved logging.

@Bai-Li-NOAA Bai-Li-NOAA added the kind: bug Something isn't working label Jun 6, 2024
@kellijohnson-NOAA kellijohnson-NOAA added P1 high priority task status: triage_needed This is not approved for this milestone, do not work on it yet labels Jun 7, 2024
@kellijohnson-NOAA kellijohnson-NOAA added this to the 2 milestone Jun 7, 2024
@Andrea-Havron-NOAA
Copy link
Collaborator

This will be refactored in the nll_refactor sprint 2 happening around the end of July. The refactor will include appropriate resizing and error checking in the C++ side and change log_obs_error to the log_sd member of respective distribution classes. In the meantime, should we leave the code as is or remove the resize call in Fleet Prepare()?

@kellijohnson-NOAA kellijohnson-NOAA removed the status: triage_needed This is not approved for this milestone, do not work on it yet label Jun 20, 2024
@Andrea-Havron-NOAA
Copy link
Collaborator

This has been fixed with the function get_force_scalar() implemented in dev. We still need to add a test to check this is working as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working P1 high priority task
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

3 participants