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 CVMix submodule #1344

Merged
merged 4 commits into from
Mar 12, 2021
Merged

Update CVMix submodule #1344

merged 4 commits into from
Mar 12, 2021

Conversation

adcroft
Copy link
Collaborator

@adcroft adcroft commented Mar 2, 2021

This PR will point the CVMix submodule to the latest tag available, and was implemented in three consecutive commits:

  1. Update to v0.93-beta with no API change
  2. Update to v0.94b-beta which has an API change and thus a MOM6 code change
  3. Update to v0.98-beta with no API change

Each commit was tested on MOM6-examples and GH actions. No answer changes were detected but MOM_tidal_mixing.F90 is not exercised in any of our tests. Providing the two intermediate commits provides finer granularity incase anyone has to git bisect for why CVMix changed answers.

I suggest @gustavo-marques have a look at this PR since NCAR is most likely to be using these modules.

- Tag v0.93-beta of CVMix is the last tag on their "master" branch before
  an API change within CVMix.
- Answers reproduce with the three-year prior commit being used, as tested
  in MOM6-examples single_column cases. These do not cover all CVMix code.
- This tag involves an API change that requires changes to MOM_tidal_mixing.F90.
- Changes are dropped arguments so presumably that data is not needed.
- Answers reproduce in so far as they are covered by MOM6-examples.
  Not all of CVMix is covered in these tests and this code in particular
  is not.
- v0.98-beta is the latest tag of CVMix that is available a.t.t.
- There were no API changes since the v0.94b-beta that affected MOM6.
- No answer changes for MOM6-examples, but these do not exercise much
  of CVMix.
@codecov
Copy link

codecov bot commented Mar 2, 2021

Codecov Report

Merging #1344 (b911a39) into dev/gfdl (250f007) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           dev/gfdl    #1344   +/-   ##
=========================================
  Coverage     45.83%   45.83%           
=========================================
  Files           234      234           
  Lines         72663    72663           
=========================================
  Hits          33302    33302           
  Misses        39361    39361           
Impacted Files Coverage Δ
...rc/parameterizations/vertical/MOM_tidal_mixing.F90 9.63% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 250f007...b911a39. Read the comment docs.

@gustavo-marques
Copy link
Collaborator

We use some of the modules touched here and I can confirm that this PR does not change answers for us.

@marshallward marshallward self-assigned this Mar 12, 2021
@marshallward
Copy link
Collaborator

Copy link
Collaborator

@marshallward marshallward left a comment

Choose a reason for hiding this comment

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

I don't see any obvious answer changes, just some refactoring and maybe some new options. So given that and no detected regressions, presumably this is safe to merge.

@marshallward marshallward merged commit 4255ada into mom-ocean:dev/gfdl Mar 12, 2021
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