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 cylc to conda dependencies #46

Merged
merged 4 commits into from
Mar 18, 2024
Merged

Conversation

ceblanton
Copy link
Collaborator

Various fre pp tools use cylc, and currently it is module loaded at GFDL.

This change would allow the fre pp tools to work at non-GFDL sites and generally easily, at the cost of a larger conda install.

Various fre tools use cylc, and currently it is module loaded at GFDL.
This change would allow the fre pp tools to work at non-GFDL sites
and generally easily, at the cost of a larger conda install.
@ceblanton
Copy link
Collaborator Author

I think we want this, for gaea and generic site use primarily (and also it eases the GFDL installation and upgrades), but it will make the conda installation bigger and slower. It's already slow, though.

@singhd789 and @Ciheim's containers conda install cylc-flow, cylc-rose, and metomi-rose, so those 3 are definitely needed.

cylc-uiserver, though, is also in the "module load cylc" environment and needed for "cylc gui". Maybe we will want to add it later, though it seems un-cli like.

@bcc2761 @chanwilson

@bcc2761
Copy link
Contributor

bcc2761 commented Mar 15, 2024

sounds good to me; we can always remove if we find a better solution

meta.yaml Outdated Show resolved Hide resolved
@singhd789
Copy link
Collaborator

In containerized post-processing world, as you mentioned Chris, we do have cylc installed in the container. If we eventually get to just a base cylc container instead of the full pp workflow, that would eliminate the installation in the fre-cli, replacing it with a container usage instead, right? Just a thought.

@ceblanton
Copy link
Collaborator Author

In containerized post-processing world, as you mentioned Chris, we do have cylc installed in the container. If we eventually get to just a base cylc container instead of the full pp workflow, that would eliminate the installation in the fre-cli, replacing it with a container usage instead, right? Just a thought.

I have to confess I'm a bit behind on the container granularity and how the fre-cli and other dependencies interact.

Is there a disadvantage to having cylc in the fre-cli envionment? It would be useful for GFDL, gaea, and generic use (without containers), but may not be useful for the containers themselves.

@singhd789
Copy link
Collaborator

singhd789 commented Mar 18, 2024

In containerized post-processing world, as you mentioned Chris, we do have cylc installed in the container. If we eventually get to just a base cylc container instead of the full pp workflow, that would eliminate the installation in the fre-cli, replacing it with a container usage instead, right? Just a thought.

I have to confess I'm a bit behind on the container granularity and how the fre-cli and other dependencies interact.

Is there a disadvantage to having cylc in the fre-cli envionment? It would be useful for GFDL, gaea, and generic use (without containers), but may not be useful for the containers themselves.

It is a conda based container and in the dockerfile, it creates a conda environment (https://github.com/NOAA-GFDL/HPC-ME/blob/86af3f664e3f07eb1df09eeb3eadef6dbf7db075/ppp/ppp-Dockerfile#L17) and uses the environment yaml (https://github.com/NOAA-GFDL/HPC-ME/blob/main/ppp/cylc-flow-tools.yaml) to install tools needed for the workflow. @Ciheim recently added the fre-cli installation into the yaml as well. So the fre-cli operates in this environment where cylc tools are already installed.

It's not a big disadvantage, and is good for a non-container use. I just had the thought that it would help the slowness of the install since it's already in the container. Since the container applies to the whole workflow at the moment though, this was a possible "in the future" thought for if we just use a cylc container.

@ceblanton
Copy link
Collaborator Author

The conda build tests are not working, but I can't make sense of the log output. @bcc2761 could you take a look sometime to see if you see the problem?

The strange part is that while the most recent build failed (https://github.com/NOAA-GFDL/fre-cli/actions/runs/8330212607),

a previous one that had the duplicate "metomi-rose" dependencies seemed to have worked (https://github.com/NOAA-GFDL/fre-cli/actions/runs/8299084818)

@singhd789
Copy link
Collaborator

The conda build tests are not working, but I can't make sense of the log output. @bcc2761 could you take a look sometime to see if you see the problem?

The strange part is that while the most recent build failed (https://github.com/NOAA-GFDL/fre-cli/actions/runs/8330212607),

a previous one that had the duplicate "metomi-rose" dependencies seemed to have worked (https://github.com/NOAA-GFDL/fre-cli/actions/runs/8299084818)

Maybe it needs the conda forge channel? - like here https://anaconda.org/conda-forge/metomi-rose

@ceblanton
Copy link
Collaborator Author

I can't disagree with you based on the evidence, but shouldn't conda-forge be available since we added in the "channels" section just above (line 16)

channels:
    - defaults
    - conda-forge
    - noaa-gfdl

I've wondered whether the prereq packages must or should include the channel.

@bcc2761
Copy link
Contributor

bcc2761 commented Mar 18, 2024

Yeah this one's interesting. mamba-org/mamba#1583 complains about the same issue, but we're not using mamba... I added a conda update conda-build here (https://github.com/NOAA-GFDL/fre-cli/actions/runs/8331516681/job/22798591714?pr=49) and the build succeeded but I'm just confused because the runner should be using the latest version of conda-build regardless; so I feel like this isn't the actual reason behind the fails

@bcc2761
Copy link
Contributor

bcc2761 commented Mar 18, 2024

^ Nevermind that, it should have failed since that action run didn't even use my PR change but instead used the current workflow script. I have 0 idea what the cause is but I'm gonna look into it more

@bcc2761
Copy link
Contributor

bcc2761 commented Mar 18, 2024

@ceblanton I just told Dana that I think these new dependencies are unrelated to the problem we're having, as this workflow also failed with the same issue, before these dependencies were added (https://github.com/NOAA-GFDL/fre-cli/actions/runs/8329589424). Trying another test run right now just to see if it's possibly fixed itself and was maybe an issue on Conda's end

@bcc2761 bcc2761 requested a review from singhd789 March 18, 2024 18:44
@singhd789
Copy link
Collaborator

Seems to have passed, I'll merge it in

@singhd789 singhd789 merged commit aa52bbf into main Mar 18, 2024
1 check passed
@bcc2761 bcc2761 deleted the add-cylc-to-conda-dependencies branch May 20, 2024 14:01
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