doc: error and progress logging for information.hpp

[nathanvaughan-NOAA]

What is the feature?

Add FIMS_LOG outputs to monitor progress and errors in information.hpp

• Added progress and error logging to information.hpp

How have you implemented the solution?

Added lots of FIMS_LOG calls throughout CreateModel() function

Does the PR impact any other area of the project?

No

Input

None

Output

Reports more details to the existing fims.log file.

How to test this change

No new tests

Developer pre-PR checklist

• I relied on GitHub actions to 🧪 things for me while I sat on the 🛋️.
• Ran cmake build and ctest locally and the C++ tests passed
• Ran devtools::check() locally and the package compiled and all R tests passed. If there are failing tests, run devtools::test(filter = "file_name") (where "test-file_name.R" is the file containing the failing tests) to troubleshoot tests.

[Cole-Monnahan-NOAA]

I was able to successfully build and test this PR locally. It outputs to fims.log in the root directory, but because output is appended it is difficult to parse and check functionality b/c its unclear if errors are expected or not. The only mandatory changes I suggest are to fix address location printing. A few suggestions for discussion/consideration:

1. File should not append from previous runs. How will this work when we run multiple FIMS models in parallel?
2. The file is very hard to read. I suggest more white space to indent like regions (e.g., all likelihood checks for a given fleet), and breaks between major sections (e.g., selex and growth).
3. I mentioned it below but I don't think printing the whole data set to the log is necessarily a good idea. I suggest we consider just printing the first and last 3 observations. It's possible something can go wrong in other years, but the user won't detect that from this file. Instead, I think that FIMS should check for invalid individual likelihood components as it calculates them and be able to error and with informative messages like "nll for age comps for fleet 1 year 35 is invalid" then print the observed and expected before exiting.

[codecov-commenter]

Codecov Report

Patch coverage is 59.75% of modified lines.

❗ Current head c8eba3c differs from pull request most recent head 98011ec. Consider uploading reports for the commit 98011ec to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Files Changed	Coverage
inst/include/common/data_object.hpp	ø
inst/include/common/information.hpp	59.75%

📢 Thoughts on this report? Let us know!.

[kellijohnson-NOAA]

Given that this is just a branch and not the main trunk, I would suggest that the commit history be fixed to just wipe out the last four commits and roll it back to 34617f5. This can easily be done with

git reset --h 34617f5
git push -f origin 407-information-logs

or through an interactive rebase which is easier with git lens where you can just choose to drop the last four commits. I am happy to do either option or walk someone through the steps so that others can learn. But in my opinion there is no reason to push four commits that we essentially do not want into main.

[nathanvaughan-NOAA]

Thanks Cole, in response to comments:

1. File should not append from previous runs. How will this work when we run multiple FIMS models in parallel?
   a) The file is no longer appending it is just running 4 iterations of create_model() one for each AD level.

2. The file is very hard to read. I suggest more white space to indent like regions (e.g., all likelihood checks for a given fleet), and breaks between major sections (e.g., selex and growth).
   a) Agree with this, I have pulled most of the other things out of the file but still need to figure out how to do useful indenting when each line of the file is written independently?

3. I mentioned it below but I don't think printing the whole data set to the log is necessarily a good idea. I suggest we consider just printing the first and last 3 observations. It's possible something can go wrong in other years, but the user won't detect that from this file. Instead, I think that FIMS should check for invalid individual likelihood components as it calculates them and be able to error and with informative messages like "nll for age comps for fleet 1 year 35 is invalid" then print the observed and expected before exiting.
   a) For now I have modified this to just print the first value of the data file. We will need to create loops to print multiple values or nans but I agree that that is probably unneeded overhead at the moment.

[ChristineStawitz-NOAA]

@Cole-Monnahan-NOAA see Nathan's response - I think you need to approve that he has addressed your comments before we can merge this in.

We are planning to fix the issue of four runs being printed after we merge in pull request #453 since the Rcpp refactor will change where it needs to be written to the log.

and thanks @kellijohnson-NOAA for giving me the emotional support and git syntax I needed to do a hard reset.