Skip to content

update package to work with SS3 version 3.30.24.2#233

Merged
nathanvaughan-NOAA merged 12 commits into
nmfs-ost:mainfrom
iantaylor-NOAA:ss3-version-3.30.24.2
Jul 10, 2026
Merged

update package to work with SS3 version 3.30.24.2#233
nathanvaughan-NOAA merged 12 commits into
nmfs-ost:mainfrom
iantaylor-NOAA:ss3-version-3.30.24.2

Conversation

@iantaylor-NOAA

Copy link
Copy Markdown
Contributor

Inspired by an email from @JuanMatiasBraccini, I asked github copilot to update the package to work with current SS3 versions using the prompt informed by issues identified in the email:

make changes to this package to get it working with the current version of SS3 (version 3.30.24.2), available via r4ss::get_ss3_exe(). known issues include change in input filenames from ss.par to ss3.par and changes to the data.ss_new output file which is now output as 3 separate files as discussed in nmfs-ost/ss3-source-code#218"

I made some additional refinements on top of what copilot provided, including

  • revising text,
  • incremented the version number from 0.3.0 to 0.4.0 (but maybe it should be 0.3.1?)
  • update SS3 executables from 3.30.18 to 3.30.24.2

The changes from copilot all look pretty good to me, but I've only looked over them very quickly.

@nathanvaughan-NOAA, let me know if you have any time or interest in reviewing these changes.

- copilot prompt: "make changes to this package to get it working with the current version of SS3 (version 3.30.24.2), available via r4ss::get_ss3_exe().

known issues include change in input filenames from ss.par to ss3.par and changes to the data.ss_new output file which is now output as 3 separate files as discussed in nmfs-ost/ss3-source-code#218"
@iantaylor-NOAA

Copy link
Copy Markdown
Contributor Author

I was able to work around an SS3 issue (nmfs-ost/ss3-source-code#764) by leaving the benchmarks on (which I don't think will do any harm) in c762193. However, I'm now seeing a problem in the integration test

devtools::test(filter = "interim")
i Testing SSMSE
Welcome to SSMSE version 0.4.0 using Stock Synthesis 3.30.24.2
v | F W  S  OK | Context
x | 2 1      2 | Integration test for interim assessment workflow [5.5s]            
------------------------------------------------------------------------------------
Warning (test-interim.R:33:3): run_SSMSE runs with interim assessment workflow
running command '"C:/github/SSMSE/inst/bin/Windows64/ss3.exe" -nohess' had status 1

Running the code from the test to create the failed model and then running the model outside of the test, I see the error:

Warning 4 Fatal Error! error on ss.par read; final value was not 999; total number parms changed  44.2094
Also see warning.sso
Exiting SS3!

I'm sure this won't be TOO hard to figure out, but I won't be able to get to it for at least a week. @nathanvaughan-NOAA, if you feel like debugging, feel free to fetch my branch and explore or just wait for me to get back to it in early July.

@iantaylor-NOAA

Copy link
Copy Markdown
Contributor Author

When complete, this pull request should fix #232.
@JuanMatiasBraccini also said he can run the models with 3.30.21 and use the status-quo SSMSE as a work around.

@nathanvaughan-NOAA

Copy link
Copy Markdown
Collaborator

Thanks @iantaylor-NOAA I'll update you if I get a chance to continue debugging.

- remove obsolete accounting for forecast imlementation error parameters
- update comments about ss.par to ss3.par
@iantaylor-NOAA

Copy link
Copy Markdown
Contributor Author

@nathanvaughan-NOAA, I fixed a few additional problems causing test failures and the tests are now passing on my computer. Can you let me know if you have time to look over the additional changes and/or enable the github actions to run automated tests? I don't have repo permissions to run them myself.

@nathanvaughan-NOAA

nathanvaughan-NOAA commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Hey @iantaylor-NOAA sorry I haven't had a chance to look through this sooner. I made a branch off main called dev-3-30-24-2 could you pull request into that branch so I can run the tests. It doesn't seem to want to let me run them on your fork. Nevermind, I was able to make a pull request from your fork and now the test are running in both pull requests anyway. I guess I fixed it somehow.

@nathanvaughan-NOAA

Copy link
Copy Markdown
Collaborator

Overall, it looks fine to me if everything is passing for you. I saw that you removed the implementation error values because you don't need them in the new version. Does that mean that this update is going to break compatibility with earlier SS versions?

@nathanvaughan-NOAA

Copy link
Copy Markdown
Collaborator

@iantaylor-NOAA for some reason the check_exe.R function is failing to find the ss3 executable in the MacOS_arm64 folder in all tests. I see that you updated the file but I have no idea why it isn't being found?

@iantaylor-NOAA

Copy link
Copy Markdown
Contributor Author

@nathanvaughan-NOAA, good point about backwards compatibility. I didn't think about that during this work and I'm sure you're right that it would break for earlier SS3 versions. I don't think it would be too hard to add some switches based on the version number, but it would add complexity to the package. In theory someone wanting to run SSMSE for an older SS3 model could just install the v0.3.0 release instead of the new version. However, I'm not in touch with the users of SSMSE so I don't know how exactly it's being applied and what combinations of SS3 versions will need to be supported going forward.

I'll look into the failed MacOS action.

@nathanvaughan-NOAA

Copy link
Copy Markdown
Collaborator

No worries @iantaylor-NOAA I just wanted to confirm these would be breaking changes. In the SE we are running a lot of older assessments which is one of the reasons we are working on our own branch that is still on 3.30.19 :)

- Co-authored-by: @Copilot
@iantaylor-NOAA

Copy link
Copy Markdown
Contributor Author

@nathanvaughan-NOAA, looks like things are passing thanks to a fix in the executable permissions suggested by copilot

@nathanvaughan-NOAA
nathanvaughan-NOAA merged commit 61d4b3a into nmfs-ost:main Jul 10, 2026
4 checks passed
@nathanvaughan-NOAA

Copy link
Copy Markdown
Collaborator

Thanks for all you work on this @iantaylor-NOAA, let us know if you have any issues with the updates @JuanMatiasBraccini

@JuanMatiasBraccini

JuanMatiasBraccini commented Jul 13, 2026 via email

Copy link
Copy Markdown

@iantaylor-NOAA

Copy link
Copy Markdown
Contributor Author

@JuanMatiasBraccini, sorry to hear that it didn't work and for the slow reply. I'm just back from a week on leave and it looks like the attachment didn't go through. You could email files to ian.taylor@noaa.gov or navigate to the github issue (#233) and drag and drop files into a comment (zip is allowed, but not .ss extensions, although they can be renamed to .txt).

@iantaylor-NOAA

Copy link
Copy Markdown
Contributor Author

@JuanMatiasBraccini, cancel that previous message. I received the attachments by your original email since it was sent to me in addition to github. I'm not sure how quickly I'll be able to work on this but hopefully in the next few days.

@JuanMatiasBraccini

JuanMatiasBraccini commented Jul 21, 2026 via email

Copy link
Copy Markdown

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

Successfully merging this pull request may close these issues.

3 participants