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

[Refactor]: Call REPORT_F from model.hpp instead of fleet and population #377

Closed
1 task done
Andrea-Havron-NOAA opened this issue Jun 20, 2023 · 4 comments · Fixed by #466
Closed
1 task done

[Refactor]: Call REPORT_F from model.hpp instead of fleet and population #377

Andrea-Havron-NOAA opened this issue Jun 20, 2023 · 4 comments · Fixed by #466
Labels
kind: bug Something isn't working kind: refactor Restructure code to improve the implementation of FIMS P1 high priority task
Milestone

Comments

@Andrea-Havron-NOAA
Copy link
Collaborator

Refactor request

Currently, derived values are reported from fleet and population. When there is more than one fleet or population, however, the vector of derived values will be overwritten. REPORT_F should be called from model.hpp instead to avoid these issues.

Expected behavior

A better structure will be to declare matrices within model.hpp, fill in the matrices within population and fleet loops and report these out directly from model.hpp. There are current issues with calling REPORT_F from model.hpp that need to be resolved.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@Andrea-Havron-NOAA Andrea-Havron-NOAA added status: triage_needed This is not approved for this milestone, do not work on it yet kind: refactor Restructure code to improve the implementation of FIMS labels Jun 20, 2023
@Andrea-Havron-NOAA Andrea-Havron-NOAA removed the status: triage_needed This is not approved for this milestone, do not work on it yet label Jun 20, 2023
@Andrea-Havron-NOAA Andrea-Havron-NOAA added this to the MQ milestone Jun 20, 2023
@Andrea-Havron-NOAA Andrea-Havron-NOAA added the kind: bug Something isn't working label Jul 11, 2023
@Andrea-Havron-NOAA Andrea-Havron-NOAA moved this to Ready for Development in MQ Jul 12, 2023
@Andrea-Havron-NOAA
Copy link
Collaborator Author

I added a call to REPORT_F in model.hpp on the 377-branch that added reporting to model.hpp to recreate the error message.

See the compilation error on the GHA

@Andrea-Havron-NOAA Andrea-Havron-NOAA moved this from Ready for Development to In Progress in MQ Jul 25, 2023
@msupernaw
Copy link
Contributor

msupernaw commented Jul 26, 2023 via email

@ChristineStawitz-NOAA ChristineStawitz-NOAA added the P1 high priority task label Aug 3, 2023
@ChristineStawitz-NOAA
Copy link
Contributor

I'm adding a P1 label to this because we report the threshold for accuracy in the FIMS manuscript and it would be good to have a standard value (needing standard errors) before we submit that.

@Andrea-Havron-NOAA
Copy link
Collaborator Author

Andrea-Havron-NOAA commented Sep 26, 2023

@timjmiller, I've used your recommendation of vector< vector > to call ragged arrays for reporting out different fleet and population derived values.

To do:

  • Fix tests
  • Do we want to reassembled dimension folded vectors back into matrices (ie. year x age) for reporting?

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 kind: refactor Restructure code to improve the implementation of FIMS P1 high priority task
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants