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

Implementing GEOS-Chem chemistry in development (CESM2-GC) #1377

Closed
wants to merge 420 commits into from

Conversation

fritzt
Copy link

@fritzt fritzt commented May 19, 2021

Hello!

I have been working on implementing the GEOS-Chem (version 13.0.0) chemistry module in CESM2.1.1. This new option is designated as CESM2-GC. The implementation of GEOS-Chem chemistry inside CESM2 aims to provide an additional chemistry option alongside CAM-Chem.

This is the second PR out of three (CAM, CLM, cime). The main PR can be found at ESCOMP/CAM#378

Description of changes

This PR focuses on passing land types and leaf area indices (LAI) from CLM to CAM through the coupler. I followed the same workflow as what is done for albedo. The land types and LAI are required in CAM to let GEOS-Chem compute its own dry deposition velocities.

Specific notes

Contributors other than yourself, if any: None

CTSM Issues Fixed (include github issue #): None

Are answers expected to change (and if so in what way)?
No, this PR introduces changes that only aim to pass information from CLM to CAM.

Any User Interface Changes (namelist or namelist defaults changes)?
No.

Testing performed, if any:
I was able to output land types and LAI as archived by CAM.


Let me know if I am targetting the wrong branch. The CLM tag I used for my work is release-clm5.0.25. Attached is the top-level Externals.cfg that I used for my project.

Regards,
Thibaud Fritz
Laboratory for Aviation and the Environment

Externals.txt

@billsacks billsacks added the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label May 19, 2021
@ekluzek
Copy link
Collaborator

ekluzek commented May 19, 2021

@fritzt thanks for the contribution.

I'm just wondering why you want this in CESM2.1 instead of the latest development version CESM2.3? CESM2.1 is considered closed in terms of development. We likely will have some continued support for it, but I didn't think that was to continue very long. And as such I would think new developments should go into the latest development code base of CESM2.3.

The other point is that the latest CESM2.1 version for CLM is release-clm5.0.34. So you should update this branch from release-clm5.0.25 to release-clm5.0.34 using git. You don't have to create a new PR, you can just do the update, commit the changes and push them to the branch and they'll show up in the PR.

ekluzek and others added 28 commits May 19, 2021 12:42
…tes from Chonggang, and added new Rogers 2014 reference
@fritzt
Copy link
Author

fritzt commented May 19, 2021

Thank you for the feedback @ekluzek!

I started out this project working on CESM2.1.1 but I'm more than happy to merge this into CESM2.3. I also believed that CESM2.1.1 was using the release-clm5.0.25 tag (as shown here https://github.com/ESCOMP/CESM/blob/release-cesm2.1.1/Externals.cfg).

I believe I have applied all the commits up to release-clm5.0.34.

@mvertens
Copy link

@fritzt - another issue to consider if you do a PR to the latest development version is that we are migrating to the NUOPC/CMEPS coupler as the default framework in the near future - so these changes also need to be implemented in the nuopc cap (in src/cpl/nuopc). I am happy to talk to you about how to do that and also help with questions. The same thing will need to be done for the new coupler (now called a mediator) and cam.

@ekluzek
Copy link
Collaborator

ekluzek commented May 19, 2021

Hmmm. So I'm seeing a huge number of differences in files that I'm doubtful that you've touched. So I'm wondering if your merge update to release-clm5.0.34 worked correctly?

@ekluzek
Copy link
Collaborator

ekluzek commented May 19, 2021

@fritzt are there CESM collaborators you are working with on this that we can talk to about this project? Maybe people in the Chemistry working group for example? Getting this on the latest release-clm5.0 is a good first start, but it still really might make the most sense for it to be on the latest development version. Doing that will take some effort, so I'd like to make sure that's the correct thing to do.

If that is the right thing to do, we should evaluate if using git is the best way to go or if moving the code into by hand into the latest version makes the most sense. If you do it in git the next merge I'd recommend would be to ctsm1.0.dev106 as that brought the release-clm5.0 changes to the development version. Depending on how that goes you might go to the very latest after that or break it up into sections.

@fritzt
Copy link
Author

fritzt commented May 19, 2021

I have been exchanging with a few people in ACOM, mostly Louisa Emmons, as well as Steve Goldhaver in CGD. Tagging @gold2718

Whatever works best! Feel free to cherry-pick my commits to any branch!

@ekluzek ekluzek changed the title Implementing GEOS-Chem chemistry in CESM2.1 (CESM2-GC) Implementing GEOS-Chem chemistry in development (CESM2-GC) May 19, 2021
@ekluzek
Copy link
Collaborator

ekluzek commented May 19, 2021

Hi @fritzt I talked with Louisa and she clarified that the plan is to bring this work into the latest development version of CESM. And there is no need to bring it into the CESM2.1 release. So I renamed the title of this to reflect that. You will need to figure out the best way to go forward for you to bring this into the development version rather than the release version. I see a couple different ways forward. As such it might be best if we have a few of us talk it over with you. I suggest starting with Louisa talk it over with her, and then setup a meeting with us in CTSM to go over your plan when needed.

@billsacks billsacks removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label May 20, 2021
@lizziel
Copy link

lizziel commented Aug 26, 2021

Hi @ekluzek, I'm Lizzie from the GEOS-Chem Support Team. I'll be working to push this through for Thibaud in the near future. Looking at the updates in the CESM-GC/CTSM branch CESM-GC_dev (not diffs in this PR) and the above conversation I think the best way forward is for me to manually add the updates to the appropriate branch. At this point I'm trying to figure out what that branch is, and pull together the framework version I should test it in. If you could give me pointers on that it would be very helpful.

@ekluzek
Copy link
Collaborator

ekluzek commented Aug 26, 2021

@lizziel this should go on what we currently call our master branch for mainline development. The last version is ctsm5.1.dev053, and you'll see that if you look at master on the second fork.

@lizziel
Copy link

lizziel commented Aug 27, 2021

Thanks, I'll open a new PR for this and close this one once it is opened.

@lizziel
Copy link

lizziel commented Aug 27, 2021

I created a local dummy PR in our CESM-GC repository just to show the changes added on top of the base tag (release-clm5.0.25). It is CESM-GC#1.

Only eight files changed. Seven of the eight no longer exist in EXCOMP/CTSM:master. The one that does has been modified such that the context of the change is no longer present.

What is the best way to proceed to figure out the new method for passing variables to CAM via the couple? Is there someone familiar with the code changes who could look at our changes to advise?

@mvertens
Copy link

@erik @lizziel - I hope that in cesm2_3_beta06 we will be moving to the nuopc/cmeps mediator as the default coupling mechanism. The mct/cpl7 caps will be phased out moving forwards. I would recommend that all new coupling in CTSM be done in the nuopc cap in src/cpl/nuopc rather than in the mct cap in src/cpl/mct. I am happy to advise on this if that would help.

@ekluzek
Copy link
Collaborator

ekluzek commented Aug 27, 2021

@lizziel this is a good question. I will look over the changes and give some guidance here. But, I think it would be worthwhile to have a virtual meeting to go over this a bit as you start this process. @mvertens is correct you should ONLY implement this in the new nuopc cap rather than the mct tag as was done here. Let me do a first pass and then I'll suggest a meeting and who should be involved on the CTSM side. @lizziel on your end please think about who should be involved in this meeting. I know the CAM-Chem people are interested in this, should Francis, Louisa, or Simone be involved in that meeting?

@ekluzek
Copy link
Collaborator

ekluzek commented Aug 27, 2021

@lizziel it took a bit to figure this out. It looks like the CESM-GC_dev2 branch is the one that has the actual changes relative to the release-clm5.0.25 version. Using that branch I see these files as having changes in them...

diff --git a/src/cpl/clm_cpl_indices.F90 b/src/cpl/clm_cpl_indices.F90
diff --git a/src/cpl/lnd_import_export.F90 b/src/cpl/lnd_import_export.F90
diff --git a/src/main/clm_driver.F90 b/src/main/clm_driver.F90
diff --git a/src/main/lnd2atmMod.F90 b/src/main/lnd2atmMod.F90
diff --git a/src/main/lnd2atmType.F90 b/src/main/lnd2atmType.F90
diff --git a/src_clm40/main/clm_atmlnd.F90 b/src_clm40/main/clm_atmlnd.F90
diff --git a/src_clm40/main/clm_cpl_indices.F90 b/src_clm40/main/clm_cpl_indices.F90
diff --git a/src_clm40/main/lnd_import_export.F90 b/src_clm40/main/lnd_import_export.F90
(So I count eight files as you mention above)

Note, that we are NOT supporting clm40 code anymore so the last three files should just be dropped. The three middle files clm_driver, lnd2atmMod and lnd2atmType are still in the same place under the main directory, so hopefully the changes there can be moved over easier. You said that you only saw one file where this was the case, so I'm a bit confused there.

The first two are the MCT coupler files which are now under src/cpl/mct, but as already talked about you need to make changes in the nuopc directory which is src/cpl/nuopc. So you'll be editing...

src/cpl/nuopc/lnd_import_export.F90

The clm_cpl_indices.F90 file isn't used for the NUOPC case.
src/cpl/nuopc/

From above I'm not sure why you said that only one of the files was in main-dev and the context changed for that file, as I'm seeing three? I haven't looked at how the context changed, but want to make sure we are both talking about the same files. I can look into the context after we make sure we are on the same page in regard to looking at the same changes. Am I right that the CESM-GC_dev2 is the one to examine?

Thanks so much for working on this. Let me know about my questions and I look forward to working with you on this...

@lizziel
Copy link

lizziel commented Aug 30, 2021

You are correct that CESM-GC_dev2 is the branch with the changes. I now see all three main/ files in the master branch. That was a lower-case "L" looking like capital "I" error on my part.

I am going to take a look at all the relevant files and I'll let you know if I have questions / want to set up a meeting. I have a meeting set up already about this work with Steve Goldhaber on Thursday. That meeting covers this and other PRs from @fritzt for cam and cime. Would it make sense for you to join?

@ekluzek
Copy link
Collaborator

ekluzek commented Aug 30, 2021

@lizziel yes joining your Thursday meeting might make sense. I'm happy to join if it works with the other meetings I have that day. If we could make the discussion of this PR first, then I could bail when you get to the other topics. We could use that meeting time as a basis of if we need another meeting with more people. Go ahead and send me the gcal invite for it, and I'll see if I can come.

@lizziel
Copy link

lizziel commented Sep 1, 2021

I created new PR #1475 to supercede this PR. Per guidance from @ekluzek I made all updates to analogous files changed in the original update with the exception of the NUOPC layer. The update for NUOPC will be made in a later commit in that PR following further discussion.

@ekluzek
Copy link
Collaborator

ekluzek commented Sep 2, 2021

@lizziel since #1475 supersedes this, I'm closing it. It'll still hang around and if for some reason we need to reopen it we can.

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.

9 participants