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

point to updated spack branch for ufs-related changes #407

Merged
merged 30 commits into from
Dec 12, 2022

Conversation

AlexanderRichert-NOAA
Copy link
Collaborator

No description provided.

@climbfuji
Copy link
Collaborator

@AlexanderRichert-NOAA error on macos-gcc for ufs-weather-model build:

[+] /Users/runner/work/spack-stack/spack-stack/envs/default/install/apple-clang/13.0.0/gftl-shared-1.5.0-jhxhcwu
==> Installing parallel-netcdf-1.12.2-gclitwdvpta75oalspcm74and2ww44ry
==> No binary for parallel-netcdf-1.12.2-gclitwdvpta75oalspcm74and2ww44ry found: installing from source
==> Fetching https://mirror.spack.io/_source-cache/archive/3e/3ef1411875b07955f519a5b03278c31e566976357ddfc74c2493a1076e7d7c74.tar.gz
==> No patches needed for parallel-netcdf
==> parallel-netcdf: Executing phase: 'autoreconf'
==> parallel-netcdf: Executing phase: 'configure'
==> Error: ProcessError: Command exited with status 2:
    '/Users/runner/work/spack-stack/spack-stack/cache/build_stage/spack-stage-parallel-netcdf-1.12.2-gclitwdvpta75oalspcm74and2ww44ry/spack-src/configure' '--prefix=/Users/runner/work/spack-stack/spack-stack/envs/default/install/apple-clang/13.0.0/parallel-netcdf-1.12.2-gclitwd' '--with-mpi=/Users/runner/mpi' 'SEQ_CC=/Users/runner/work/spack-stack/spack-stack/spack/lib/spack/env/clang/clang' '--enable-cxx' '--enable-fortran' 'CFLAGS=-fPIC' 'CXXFLAGS=-fPIC' 'FCFLAGS=-fPIC' 'FFLAGS=-fPIC' '--enable-relax-coord-bound' '--enable-shared' '--enable-static' '--disable-silent-rules'

1 error found in build log:
     130    checking for C++ compiler vendor... clang
     131    checking base compiler command in MPICXX wrapper... not found
     132    checking for MPI_File_close... yes
     133    checking whether MPI C++ compiler redefines SEEK_SET ... no
     134    checking whether /Users/runner/mpi/bin/mpif77 exists and is executa
            ble... yes
     135    checking whether /Users/runner/mpi/bin/mpif90 exists and is executa
            ble... yes
  >> 136    /Users/runner/work/spack-stack/spack-stack/cache/build_stage/spack-
            stage-parallel-netcdf-1.12.2-gclitwdvpta75oalspcm74and2ww44ry/spack
            -src/configure: line 23095: syntax error near unexpected token `('
     137    /Users/runner/work/spack-stack/spack-stack/cache/build_stage/spack-
            stage-parallel-netcdf-1.12.2-gclitwdvpta75oalspcm74and2ww44ry/spack
            -src/configure: line 23095: `     ## test no != "$_LT_TAGVAR(hardco
            de_shlibpath_var, F77)" &&'

See build log for details:
  /Users/runner/work/spack-stack/spack-stack/cache/build_stage/spack-stage-parallel-netcdf-1.12.2-gclitwdvpta75oalspcm74and2ww44ry/spack-build-out.txt

See https://github.com/NOAA-EMC/spack-stack/actions/runs/3527458921/jobs/5916545108

@climbfuji
Copy link
Collaborator

@AlexanderRichert-NOAA I would like to merge this PR (and the spack PR) after #408 - that PR has its spack submodule PR merged already and just requires a final test on an HPC (doing now). CI tests are ok. You'll need the CI test bug fixes from #408 for your's to work.

@AlexanderRichert-NOAA
Copy link
Collaborator Author

AlexanderRichert-NOAA commented Nov 29, 2022 via email

@climbfuji
Copy link
Collaborator

One of the CI tests seem to have failed for no obvious reason: https://github.com/NOAA-EMC/spack-stack/actions/runs/3579347796/jobs/6023518324 - I am rerunning the test, and if successful I'll run the other canceled jobs manually, too.

Copy link
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The submodule pointer update isn't correct, in fact it's not there at all. You are pointing to the jcsda_emc_spack_stack branch in the noaa-emc fork. In addition to updating .gitmodules, you also need to commit the spack hash (which points to your branch ufs_hera_update in your fork, hash is currently 5bfe007b90faa47c88daa6b1d56a5b3be7d2886f).

@climbfuji
Copy link
Collaborator

@AlexanderRichert-NOAA Something is wrong with this PR, I don't see the submodule pointer update for your spack PR. I see only the .gitmodules change.

@climbfuji
Copy link
Collaborator

The issue with the CI tests is that spack now builds two versions of netcdf (shared) that only differ in their parallel-netcdf variant:

spec: netcdf-c@4.8.1%apple-clang@13.0.0+cdf5+dap~doxygen~fsync~hdf4~jna+largefile+mpi+netcdf-4+optimize~parallel-netcdf+parallel-tests+pic+shared+utilities+v2 build_system=autotools patches=de556da arch=darwin-bigsur-ivybridge
spec: netcdf-c@4.8.1%apple-clang@13.0.0+cdf5+dap~doxygen~fsync~hdf4~jna+largefile+mpi+netcdf-4+optimize+parallel-netcdf+parallel-tests+pic+shared+utilities+v2 build_system=autotools patches=de556da arch=darwin-bigsur-ivybridge

@climbfuji
Copy link
Collaborator

I am seeing the same problem on my macOS with apple-clang, and on Cheyenne with Intel. Each time when building from the skylab-dev template.

@AlexanderRichert-NOAA
Copy link
Collaborator Author

I am seeing the same problem on my macOS with apple-clang, and on Cheyenne with Intel. Each time when building from the skylab-dev template.

Hmm yep, that makes sense, I should have seen this coming. I think the play is to just take base-env out of ufs-weather-model-env and just have all the (non-python) UFS-related packages in one place, that way no conflicts over particular needs about netcdf variants etc. I'll update this shortly.

@climbfuji
Copy link
Collaborator

I am seeing the same problem on my macOS with apple-clang, and on Cheyenne with Intel. Each time when building from the skylab-dev template.

Hmm yep, that makes sense, I should have seen this coming. I think the play is to just take base-env out of ufs-weather-model-env and just have all the (non-python) UFS-related packages in one place, that way no conflicts over particular needs about netcdf variants etc. I'll update this shortly.

I think I have a better solution in place, I messaged you in slack but I can also post it here. We can tag up quickly in google meet to discuss this if you like.

I think we need to remove almost all the variant specifiers in base-env, except the +/~shared logic. I’ll try that and send you a PR.

Please have a look at JCSDA/spack@c33b8ec in conjunction with 73a71f8. I think this is all you need on top of your PR.

10:33
I am testing this now. If that works, how about the following suggestion: We review each other’s PRs and have other people review them individually. but then we only focus on merging JCSDA/spack#203 and #416 . This will automatically mark all other PRs that I have pulled in as merged. We’ll only need to test (CI + manual testing) #416 and make sure it does what it should.

@climbfuji
Copy link
Collaborator

@AlexanderRichert-NOAA @mathomp4 I am happy with this PR and the associated spack PR. Please open this PR up for review, and if @mathomp4 could take another look at JCSDA/spack#192, please?

@climbfuji
Copy link
Collaborator

Note on CI testing. CI tests all passed for 2b4a2a4 - including on my macOS (used the github label to kick off the self-hosted runner).

@AlexanderRichert-NOAA AlexanderRichert-NOAA marked this pull request as ready for review December 12, 2022 16:56
@climbfuji
Copy link
Collaborator

@AlexanderRichert-NOAA I merged the spack PR. Please point your spack submodule pointer in this spack-stack PR back to the head of noaa-emc jcsda_emc_spack_stack (correct hash is 377817d) and revert the .gitmodules changes. No need to run the CI tests again.

@AlexanderRichert-NOAA
Copy link
Collaborator Author

@climbfuji Done

Copy link
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified the spack submodule hash, correct.

.gitmodules Outdated Show resolved Hide resolved
@climbfuji climbfuji merged commit 2b7100c into JCSDA:develop Dec 12, 2022
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