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

update Manifest to use registered ClimaLSM #94

Closed
wants to merge 2 commits into from

Conversation

kmdeck
Copy link
Contributor

@kmdeck kmdeck commented Jul 18, 2022

PULL REQUEST

Purpose and Content

Upgrade Manifest to use ClimaLSM - registered version

Note that when I went to do so, there was a conflict between the IntervalSets compat in ClimaAtmos and ClimaLSM. I am broadening the compat in ClimaLSM to overlap with ClimaAtmos. That is the branch that is currently used in this PR. But, I'd like to wait to release another version of ClimaLSM until there are more changes, so this is on hold.

@kmdeck
Copy link
Contributor Author

kmdeck commented Jul 18, 2022

@jb-mackay @LenkaNovak for visibility - will return to this once we release another ClimaLSM.

@kmdeck kmdeck changed the title update Manifest to use hotfix ClimaLSM update Manifest to use registered ClimaLSM Jul 18, 2022
@jb-mackay
Copy link
Contributor

@kmdeck what other dependencies are requiring us to check in the Manifest for this experiment? If just LSM, we could remove the Manifest as long as the compat versions are specified in the Project.toml

@kmdeck
Copy link
Contributor Author

kmdeck commented Jul 19, 2022

we actually can use the current registered ClimaLSM if we upgrade the version of atmos we are using (I think) - I saw a PR in ClimaAtmos that bumps the compat of CA to include 0.7 for Interval sets, which is what ClimaLSM uses.

We may not need to check in the Manifest, Im not sure. Would the above still cause an issue, though? If CA and CLSM have compat entries that do not overlap?

@LenkaNovak
Copy link
Collaborator

Outdated and addressed in #143

@LenkaNovak LenkaNovak closed this Nov 6, 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