search

nmfs-ost / ss3-source-code

The source code for Stock Synthesis (SS3).
https://nmfs-ost.github.io/ss3-website/
Creative Commons Zero v1.0 Universal
36 stars 16 forks source link

Update workflows admb docker #568

Closed e-perl-NOAA closed 6 months ago

e-perl-NOAA commented 6 months ago

Concisely describe what has been changed/addressed in the pull request.

Update all workflows to use the admb docker image.

What tests have been done?

Where are the relevant files?

What tests/review still need to be done?

See PR #552

Is there an input change for users to Stock Synthesis?

Additional information (optional).

e-perl-NOAA commented 6 months ago

@Rick-Methot-NOAA please look at the new warning_ss_ref.txt file. There are some warnings produced by the admb docker image that we haven't seen prior but look like they might be easy fixes.

Rick-Methot-NOAA commented 6 months ago

I saw one warning regarding an unguarded "For". I will fix that.

Note that For statements that rely on indentation for interpretation are easily broken. I try to always use explicit {}.

e-perl-NOAA commented 6 months ago

@iantaylor-NOAA my thought process for retaining them is to have them easily accessible in case 1) the docker image stops working and 2) we need to test compiling locally across all/multiple OS' (in the case that admb no longer will compile SS3 locally and we need to see if it's a compiler issue or if we need to see if it will compile locally when we finally get updated to windows 11).

Rick-Methot-NOAA commented 6 months ago

Surely someone is already compiling with Win11. Is there significant risk?

e-perl-NOAA commented 6 months ago

I'm not sure at this point. I can delete the commented-out portion that downloads admb and puts it in the path for most of the workflows but leave it in one (probably the build-ss3) just to have easily accessible as a just in case measure, at least until we have a better understanding of what complications could arise?

Rick-Methot-NOAA commented 6 months ago

I think it makes sense to archive it in some readily retrievable form. The source code repo has lots of commits, so if we rely on git history to retrieve this, then we at least should have a label on that commit.

iantaylor-NOAA commented 6 months ago

Given this background, I think it's fine to leave the commented-out code in the files for now. If we find that we haven't needed down the road they can always be deleted later.

e-perl-NOAA commented 6 months ago

Okay sounds good. I'll go ahead and merge then!