search

James-Thorson-NOAA / FishStatsUtils

Shared resources for spatio-temporal models
GNU General Public License v3.0
10 stars 16 forks source link

Changes recently introduced in make_spatial_info in the development branch are computationally intensive #17

Closed agruss2 closed 5 years ago

agruss2 commented 5 years ago

@James-Thorson @James-Thorson-NOAA I just wanted to warn you about an issue with the function FishStatsUtils::make_spatial_info as currently coded in the development branch. Today, I needed to work with a spatio-temporal model for the Eastern Bering Sea using 300 uniformly distributed knots. The R code for that model starts by installing the latest versions of TMB, VAST and the development version of FishStatsUtils, if those have been recently updated.
The issue I had today was that, when I reached the line where the function make_spatial_info is implemented, my laptop froze and I had to restart it. This issue was solved when I replaced:

devtools::install_github("James-Thorson-NOAA/FishStatsUtils", ref="development")

with:

devtools::install_github("James-Thorson-NOAA/FishStatsUtils")

It seems that you updated the function make_spatial_info in the development version of FishStatsUtils six days ago, and that the changes that you introduced in the function make_spatial_info are computationally intensive. Therefore, you may want to revise these changes before committing them to the main branch of FishStatsUtils.

Many thanks!

James-Thorson-NOAA commented 5 years ago

I'll probably wait for more feedback before pursuing this -- but if you'd like more immediate assistance to get the development branch working for your situation, could you please run individual lines of make_spatial_info to determine what specific change in that function caused the change?

James-Thorson-NOAA commented 5 years ago

I confirmed the issue using the "simple example" -- it was occurring for a few regions due to changes on the development branch in how a default value for flip_around_dateline was being passed to make_extrapolation_info. Long story short, it should now be fixed on the development branch, and thanks for flagging it.