errors in gha latest Mac run due to no arm64 architecture binary
k-doering-NOAA opened this issue · 15 comments
The latest scheduled run of r cmd check found errors when running tests on mac. It looks like these are all related to r4ss::SS_readdat() (or many of them are, I just did a quick scroll through).
@iantaylor-NOAA or @e-perl-NOAA do you know if there were changes to r4ss in the past week that could have caused this? The SSMSE run was passing 1 week ago, so I think this must be caused by a recent change. Just noting again that it only is showing up on Mac and no other OSes, for some reason.
EDIT: Edited the title to reflect the problem @e-perl-NOAA identified further down in the thread.
the macos-latest is now macos-14 which is has an arm64 architecture and requires a different executable. There is now a version of this on the ss3 3.30.22.1 release. Also, the default language for a lot of stuff, including calling executables is to use "ss3" rather than "ss" (including the ss.par file created is now named ss3.par). Idk if these are the source of the issues but updating/changing these accordingly in ssmse might cut down on them or even hopefully solve them!
Aha, that is probably the issue!
SSMSE is designed to run with SS3.30.18. I think there are 2 paths here:
- recompiling a version of SS3.30.18 with the arm64 architecture, leave the github action as is
- run github actions with an older mac osx version.
Do you think users are likely to need the arm64 architecture? If so, 1) seems like the better option, I think!
@k-doering-NOAA, sorry for the headache.
If SSMSE is using 3.30.18, I don't see how changes in the SS3 releases available for macos would impact the tests.
Also, the r4ss::SS_readdat* functions haven't been updated in a long time and I also don't see how they would work differently on a Mac unless somehow the data file it's trying to read isn't getting created by the executable.
It's possible that some update to dependent packages or other behind the-scenes elements of the github actions are causing the failure. Running the test step by step in a codespace or replicating locally on a mac might be necessary to debug.
You could also try re-running the failed github action just in case that fixed the problem (it often does for package installation errors for me, but maybe not for an R error like this).
Kathryn, can you try using this exe - ss3-macos-14.zip
Yep, using that exe allowed the workflow to run just fine!
@e-perl-NOAA did ss3sim deal with this yet? Do you just need 2 versions of mac executables for people depending on the architecture needed?
Sorry for being confused about this. I misinterpreted the conclusion as the release of the macos-14 version of SS3 broke something when instead it was the change in the github action runners that broke things and @e-perl-NOAA was ready with a fix.
Yes, it appears that the executables compiled for each of the two architectures are not compatible with the other.
If we retroactively add the new ss3-macos-14 executable to the SS3 version 3.30.18 release then the ss3sim package and/or tests could theoretically be modified to use r4ss::get_ss3_exe(version = 'v3.30.18') instead of depending on executables included with the package. This solution would also require modifying the warning about "ss3 executable not available for macOS arm64 architecture computers for versions prior to v.3.30.22.1" in https://github.com/r4ss/r4ss/blob/main/R/get_ss3_exe.r#L92-L93. But maybe that's solving a problem that doesn't yet exist if there aren't yet SSMSE users using macos-14.
No ss3sim has not dealt with this yet. It's been a new development in the past 2-3 weeks that we needed to make this new exe for arm64 macs. @kellijohnson-NOAA
If SSMSE and ss3sim rely on older versions of ss3, I can create the exes for those. It's a bit convoluted because I have to created a new branch from each previous release tag, add the new workflow file, and then push to get it to run since you can't run a new workflow on an previous release tag (I googled this for a long time to make sure) but I'm happy to do it if needed!
@e-perl-NOAA , if it's ok to create just an SS3.30.18 binary with the Mac arm64 architecture, I think that would be great! I don't see SSMSE getting updated to a new version of SS3 anytime soon. However, I think there is no rush on this, as @iantaylor-NOAA pointed out, it is likely not an issue for users yet.
@e-perl-NOAA and @iantaylor-NOAA, I like the idea of having a good workflow for not distributing executables via the package, but I'm not sure if I have time to work on it right now.
@k-doering-NOAA the exe that I built and attached in an earlier comment is the v3.30.18 exe for arm64 macs. I'm happy to post that to the ss3 release for v3.30.18 but in doing that it would make sense to update the r4ss function get_ss3_exe() which would be a bit tricky since there would be no other arm63 exes between v3.30.18 and 3.30.22.1. Or I would need to make exes for all previous releases between v.3.30.18 and 3.30.33.1. Do you or @iantaylor-NOAA have any preferences on the best way to move forward with this that meets your needs?
@e-perl-NOAA, I'm fine with whatever you think is the best course of action here. I understand not wanting to add new binaries to older SS3 releases. I'm also personally fine if the binary were added, but get_ss3_exe() were not updated, but I'm not sure what @iantaylor-NOAA thinks about this.
If a new binary is not added, I can just update our GitHub action for SSMSE to use an older mac runner for now.
Sorry for not chiming in on this last week. Since @e-perl-NOAA has already built the binary, I think that adding it to the release makes sense for the sake of SSMSE and see no need to add it to any other old release.
I think that adding it to the 3.30.18 release should make r4ss::get_ss3_exe() work for that one version with no changes to the R code because it will find the URL instead of triggering the error here: https://github.com/r4ss/r4ss/blob/main/R/get_ss3_exe.r#L86-L106. The message "ss3 executable not available for macOS arm64 architecture computers for versions prior to v.3.30.22.1" will no longer strictly be true, but I don't think we want to encourage users to track down that specific old version by modifying the message.
Thanks @iantaylor-NOAA , that sounds like a great solution to me, if it sounds ok to @e-perl-NOAA !
Okay, the arm64 exe has been added to the ss3 v3.30.18 release.
Thanks @e-perl-NOAA for adding the arm64 exe!