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

SS3 update to .ss_new naming convention #167

Open
CassidyPeterson-NOAA opened this issue Nov 30, 2023 · 8 comments
Open

SS3 update to .ss_new naming convention #167

CassidyPeterson-NOAA opened this issue Nov 30, 2023 · 8 comments
Assignees

Comments

@CassidyPeterson-NOAA
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

@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
Copy link
Collaborator

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
Copy link
Collaborator

Thanks @nathanvaughan-NOAA !

@k-doering-NOAA
Copy link
Collaborator

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

@CassidyPeterson-NOAA
Copy link
Collaborator Author

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
Copy link
Collaborator

@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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants