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

Add new template cylc-dev (and test building in GitHub actions) #1410

Merged

Conversation

climbfuji
Copy link
Collaborator

@climbfuji climbfuji commented Dec 11, 2024

Summary

This PR adds a new template cylc-dev to build a special environment for running cylc. Because of the way cylc works, this is best accomplished by creating an environment view and then a cylc wrapper that contains the minimum settings necessary to use cylc from the view's bin directory. See cylc/cylc-flow#6532 for more information. Note that the creation of the cylc wrapper is not part of this PR.

Associated changes:

  • To compile cylc-dev on the Ubuntu self-hosted runner, exclude meson from the spack external find call. Spack found a deprecated version [email protected] and despite it being deprecated it tried to use it - with the result that the harfbuzz build failed (needed for cylc-dev). Letting spack build meson (it picks [email protected]) solved the problem.
  • Remove the +excel variant from py-pandas, which was something the jedi-tools-env used, but that is no longer needed and that caused build errors (and added several more dependencies).
  • Fix a bug in the GitHub actions that would allow the tests to fail silently (replace set +e with set -e).
  • To avoid duplicate packages, require py-cython@3 and [email protected]
  • With Intel Classic and LLVM, build qt with gcc (has no effect on pre-configured sites, where qt is configured as an external package)

Testing

  • CI (also tests cylc-dev template with gcc on Ubuntu)
  • Atlantis (built cylc-dev with gcc, created cylc wrapper, tested flow, gui and NEPTUNE workflow)
  • Narwhal (built cylc-dev with gcc, created cylc wrapper, tested flow, gui)
  • Nautilus (built cylc-dev with gcc, created cylc wrapper, tested flow, gui)

Applications affected

None

Systems affected

None

Dependencies

Issue(s) addressed

Resolves #1413
Resolves #1444

Checklist

  • This PR addresses one issue/problem/enhancement, or has a very good reason for not doing so.
  • These changes have been tested on the affected systems and applications.
  • All dependency PRs/issues have been resolved and this PR can be merged.

…template, disable cycl variant in skylab-dev template
@climbfuji climbfuji self-assigned this Dec 11, 2024
@climbfuji climbfuji added the NAVY United States Naval Research Lab label Dec 11, 2024
@climbfuji
Copy link
Collaborator Author

Note. For ede071b, GitHub actions reports that the tests succeeded, but in fact the build of cylc-dev failed: https://github.com/JCSDA/spack-stack/actions/runs/12662753274/job/35288186039?pr=1410

@climbfuji climbfuji requested a review from eap January 8, 2025 22:58
@climbfuji climbfuji force-pushed the feature/unpin_update_common_packages_PLUS_cylc branch from 080d852 to 3f79cfc Compare January 10, 2025 00:57
Copy link
Collaborator

@eap eap left a comment

Choose a reason for hiding this comment

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

Thanks for this, based on description I'm still a little puzzled, I thought we were also intending to have a cylc meta module that could be loaded along side of other modules as in our unified dev environment.

I'm approving because this code looks fine and I assume you'll have a good response to my question above.

@@ -111,7 +111,7 @@ jobs:
set +e
spack mirror add local-binary file:///Users/ec2-user/spack-stack/build-cache/
spack buildcache update-index local-binary || (echo "No valid binary cache found, proceed without" && spack mirror rm local-binary)
set +e
set -e
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch here and elsewhere.

# Variant ~mpi not working for latest py-netcdf4
# https://github.com/spack/spack/issues/47652
py-netcdf4:
require: '@1.7.1 +mpi'
py-pandas:
require: '+excel'
# To avoid duplicate packages
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't the concretizer unify: true prevent duplication?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It will lead to concretization errors if unify: true is set. Unfortunately, the concretizer isn't perfect, it still has its bugs. But I am happy to move this (numpy and cython) into the cylc-dev template instead of applying it to all environments for now (but new packages that are coming down the road will want the same settings for unified-env/neptune-env, too).

@climbfuji
Copy link
Collaborator Author

Thanks for this, based on description I'm still a little puzzled, I thought we were also intending to have a cylc meta module that could be loaded along side of other modules as in our unified dev environment.

I'm approving because this code looks fine and I assume you'll have a good response to my question above.

Initially, the idea was to add cylc to the unified-env and other larger environments. But, as we are expanding the use of Python in our environments (more and more packages with complicated dependencies), this is getting harder and harder.

Especially cylc has so many tight version requirements that it's impossible to build it in unified-env without duplicate packages ( (and is very hard to build with anything else but gcc). Then you also need the view+wrapper (wrapper is currently a manual process) steps to make it work with CYLC_PYTHONPATH and keep the environment to run cylc in and the one that cylc tasks use completely separate. This is a non-trivial setup to get right, and the link to the cylc issue/discussion is only the entry point. I've come to learn that cylc is designed to operate in its own, separate environment (which does make a lot of sense), and this can be done best with two separate spack-stack environments.

@climbfuji climbfuji merged commit 2470fb4 into JCSDA:develop Jan 10, 2025
1 check passed
@climbfuji climbfuji deleted the feature/unpin_update_common_packages_PLUS_cylc branch January 10, 2025 22:11
climbfuji added a commit that referenced this pull request Jan 14, 2025
This is a follow-up for #1410 and provides the necessary information for building and using cylc environments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
4 participants