Adds area to get_results_timeseries() for SSmse

[kellijohnson-NOAA]

I am so excited that the user community wants to add area-specific models to the mix! So, thank you @skylersagarese-NOAA for reaching out. Hopefully this change fixes the lack of area in the time series results for you. Given the slightly complicated workflow here, you will need to install this branch of ss3sim and then run your code for SSmse as we talked. The branch is locally passing the tests but there are 4 warnings related to things that I need to update and do not pertain, or at least I do not think that they pertain, to the code changes for this request. Again, thanks for reaching out and let me know how your testing goes. You can also test the function locally by just installing this branch and running

output_list <- r4ss::SS_output("YourDirectoryWithModelFilesHere", verbose = FALSE, printstats = FALSE)
small_test <- ss3sim::get_results_timeseries(output_list)
head(small_test)

Once you approve @skylersagarese-NOAA, then I will assign @k-doering-NOAA as a reviewer.

[skylersagarese-NOAA]

It worked. Area is now retained in the summary function (SSMSE borrowing from ss3sim), so we can sum TS values across areas by year on the back end for our purposes. Thank you @kellijohnson-NOAA !

[kellijohnson-NOAA]

Thanks @skylersagarese-NOAA for testing it out!

@k-doering-NOAA I have assigned you as a reviewer to make sure that I did not break anything for SSmse. Please feel free to say that you do not have time.

[k-doering-NOAA]

Thanks @kellijohnson-NOAA and @skylersagarese-NOAA , I can review!

I am testing the branch to make sure no tests in SSMSE are broken by this change: https://github.com/nmfs-fish-tools/SSMSE/tree/test-ss3sim

[k-doering-NOAA]

@kellijohnson-NOAA unfortunately this breaks some code in SSMSE (I think because of the new added column). I won't have time to work on fixes until Friday, is it ok to leave this as an on PR until then?

[kellijohnson-NOAA]

Yes, that is just fine, @skylersagarese-NOAA said that there was no rush.

[kellijohnson-NOAA]

@k-doering-NOAA There is the following failed test in test-runSSMSE.R that is not failing b/c of ss3sim

  # change one of the summary values so that the SSB_ratio > 2
  summary$ts[4, "SpawnBio"] <- (summary$ts[4, "SpawnBio"])^2 # make really large
  expect_warning(
    ssb_check_warn <- check_convergence(summary, min_yr = 101, max_yr = 106),
    "Some large"
  )
  expect_true(ssb_check_warn[1, "SSB_ratio"] > 2)

because summary$ts[4] is not within years 101--106 so the expected warning is not generated and the returned object ssb_check_warndoes not actually contain the year that you altered withsummary$ts[4,"SpawnBio"]`. But, I am not sure how this test is passing in the main branch right now.

If I change the test in expect_warning to

ssb_check_warn <- check_convergence(summary, min_yr = 4, max_yr = 106)

Then both of the checks pass.

[k-doering-NOAA]

@kellijohnson-NOAA, I think you are right that the indices of the summary$ts object in the test changed with this change in ss3sim, but this is just an issue of the test that was too fragile, not a problem in ss3sim.

Feel free to merge this in when you are ready!

[kellijohnson-NOAA]

Thanks @skylersagarese-NOAA for the suggestion and following through with the testing. Also, thanks to @k-doering-NOAA for testing within SSmse!