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 CFC11, CFC12 tracers #4552

Merged
merged 4 commits into from
Sep 29, 2021
Merged

add CFC11, CFC12 tracers #4552

merged 4 commits into from
Sep 29, 2021

Conversation

maltrud
Copy link
Contributor

@maltrud maltrud commented Sep 22, 2021

This PR adds the capability for the ocean to simulate CFC11 and CFC12 as passive tracers following the OCMIP protocol (http://ocmip5.ipsl.jussieu.fr/OCMIP/phase2/simulations/CFC/HOWTO-CFC.html). This capability is inactive by default and requires a time series of annual average atmospheric mole fractions to be read in as a new stream in streams.ocean.

[NML]
[BFB]

@xylar
Copy link
Contributor

xylar commented Sep 23, 2021

@maltrud, I'm happy to review this. So far, BGC support in compass is very limited and, from what you have told me, significantly out of date or at least out of sync with your workflow. I can easily run some "do-no-harm" tests in compass. But I'm open to suggestions for how I can test this more rigorously.

It would be really good to get some up-to-date BGC tests into compass so these parts of the code get exercised on a more regular basis. That would take someone dedicating a good chunk of time to learning compass or me dedicating time to getting spun up on BGC, so not something that will happen without some buy-in from you and others working on BGC.

@maltrud
Copy link
Contributor Author

maltrud commented Sep 23, 2021

@xylar i think for now the "do no harm" path is sufficient. it certainly worked with ideal age where i actually did some harm. we can consider more BGC (or more generally, passive tracers) wrt compass in the future.

@xylar
Copy link
Contributor

xylar commented Sep 23, 2021

Okay, sounds good.

@mark-petersen
Copy link
Contributor

With last commit, tested with gnu and intel on grizzly, optimized and debug, with CFCs off. Passes pr suite, matches compass baselines for optimized versions (except didn't test cosine bell because it takes too long).

@maltrud can you explain your testing with CFCs on? That can compliment my 'do no harm' testing with CFCs off.

Copy link
Contributor

@xylar xylar left a comment

Choose a reason for hiding this comment

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

I tested compass (without CFCs) using the pr test suite on Chrysalis with intel and impi. All results (including the cosine bell convergence test) were bit-for-bit with the most recent benchmark.

I do not feel qualified to do a more extensive review of the code itself. At a glance, it seems reasonable but I do not know very much about how CFCs are to be treated in MPAS-Ocean.

@maltrud
Copy link
Contributor Author

maltrud commented Sep 23, 2021

@mark-petersen i tested with CFCs on in several G-cases using the EC30to60r2 mesh. i ran for 60 years (1958-2007) and compared to observations and results from CESM to confirm that the code appears to be working as expected.

@rljacob rljacob added this to the v2.1alpha milestone Sep 28, 2021
Copy link
Contributor

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

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

Thanks @maltrud. Along with tests last Thursday, I merged to today's master and it passes:

SMS_D_Ln9.T62_oQU120_ais20.MPAS_LISIO_TEST.chrysalis_gnu
SMS_D_Ln9.T62_oQU120_ais20.MPAS_LISIO_TEST.chrysalis_intel
PET_Ln3.T62_oEC60to30v3wLI.GMPAS-DIB-IAF-ISMF.chrysalis_gnu

A side note: ERS_Ld3.T62_oQU240.GMPAS-IAF.chrysalis_gnu fails both master and this PR.

jonbob added a commit that referenced this pull request Sep 28, 2021
Add CFC11, CFC12 tracers

This PR adds the capability for the ocean to simulate CFC11 and CFC12 as
passive tracers following the OCMIP protocol. This capability is inactive
by default and requires a time series of annual average atmospheric mole
fractions to be read in as a new stream in streams.ocean.

[NML]
[BFB]
@jonbob
Copy link
Contributor

jonbob commented Sep 28, 2021

passed e3sm_developer on chrysalis with expected NML DIFFs, merged to next

@jonbob jonbob merged commit 50e4973 into master Sep 29, 2021
@jonbob
Copy link
Contributor

jonbob commented Sep 29, 2021

merged to master and expected NML DIFFs blessed

@jonbob jonbob deleted the maltrud/ocean/add-cfc-tracers branch September 29, 2021 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants