nmfs-fish-tools / SSMSE

Management Strategy Evaluation (MSE) using Stock Synthesis (SS3)
https://nmfs-fish-tools.github.io/SSMSE
MIT License
18 stars 11 forks source link

SS3 update to .ss_new naming convention #167

Open CassidyPeterson-NOAA opened 9 months ago

CassidyPeterson-NOAA commented 9 months ago

SS3 v3.30.19.01 renames new data file as data_echo.ss_new (instead of previously named data.ss_new). This triggers an error in the function SSMSE:::copy_model_files(). See lines of code below.

if (!all(c("control.ss_new", "data.ss_new", "starter.ss_new", 
            "forecast.ss_new", "ss.par") %in% list.files(OM_in_dir))) {
            stop(".ss_new files not found in the original OM directory ", 
                OM_in_dir, ". Please run the model to make the .ss_new files available.")
        }
nathanvaughan-NOAA commented 9 months ago

Thanks for finding this @CassidyPeterson-NOAA , a quick check of the code finds data.ss_new calls in a bunch of other places too so this is probably a wider issue.

k-doering-NOAA commented 9 months ago

@CassidyPeterson-NOAA , thanks, I think this isn't caught because SSMSE was developed using 3.30.18.

Is a newer version of SS3 necessary for the work you are doing?

For a fix, I think we could generalize the code to expect either data.ss_new or data_echo.ss_new

nathanvaughan-NOAA commented 9 months ago

I'm running through the code quickly and adding switches wherever we call data.ss_new at the moment. For example

if(file.exists(file.path(OM_dir, "data.ss_new"))){ exp_vals <- r4ss::SS_readdat(file.path(OM_dir, "data.ss_new"), section = 2, verbose = FALSE ) }else if(file.exists(file.path(OM_dir, "data_echo.ss_new"))){ exp_vals <- r4ss::SS_readdat(file.path(OM_dir, "data_echo.ss_new"), section = 2, verbose = FALSE ) }else{ stop("Error: No data.ss_new file or data_echo.ss_new file was found.") }

I'll push it up as a new branch for @CassidyPeterson-NOAA to test once I'm done. In the future we could do a better job by setting the base file names such as data.ss_new vs data_echo.ss_new at the start of the code but this will get us running for now.

k-doering-NOAA commented 9 months ago

Thanks @nathanvaughan-NOAA !

k-doering-NOAA commented 7 months ago

@CassidyPeterson-NOAA do you think it is still worth doing this, if the simulations will be run with 3.30.18?

CassidyPeterson-NOAA commented 7 months ago

It may not be a priority, since it is easily fixable by manually renaming input files ahead of running the model.

However, I think we could easily fix it by adding 2 lines of code to the copy_model_files() function:

copy_model_files <- function(OM_in_dir = NULL,
                             OM_out_dir = NULL,
                             EM_in_dir = NULL,
                             EM_out_dir = NULL,
                             verbose = FALSE) {
  file.exists(file.path(OM_in_dir))

## Following two lines added to ID if data_echo.ss_new exists, and if so, to rename a copy of the file as data.ss_new
  if("data_echo.ss_new" %in% list.files(OM_in_dir)){file.copy(file.path(OM_in_dir, "data_echo.ss_new"), file.path(OM_in_dir, "data.ss_new"), overwrite=TRUE)} 
  if("data_echo.ss_new" %in% list.files(EM_in_dir)){file.copy(file.path(EM_in_dir, "data_echo.ss_new"), file.path(EM_in_dir, "data.ss_new"), overwrite=TRUE)}
## end addition 

  # checks
  if (!is.null(OM_in_dir)) { ... }
... 
}

I made the change locally and am running diagnostic tests. Let me know if you'd like me to make another PR to incorporate this addition.

k-doering-NOAA commented 7 months ago

@CassidyPeterson-NOAA if you have time for it, a pull request would be fantastic!

There may be some other data.ss_new use in the code (for example, when we sample from the om, we are running the SS3 bootstrap procedure, and pull the samples from the data.ss_new file, I believe). I think the .19 approach is splitting those to a separate file.

However, I'm also happy if you want to put your solution into a PR, and then I can add to it, looking for the other instances of data.ss_new in the code!

k-doering-NOAA commented 5 months ago

I think #190 is caused by the new naming conventions, also.