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

Drydep issue with FATES-NOCOMP in coupled model #77

Open
kjetilaas opened this issue Aug 23, 2024 · 14 comments
Open

Drydep issue with FATES-NOCOMP in coupled model #77

kjetilaas opened this issue Aug 23, 2024 · 14 comments
Milestone

Comments

@kjetilaas
Copy link
Collaborator

from @mvdebolskiy:

DryDepVelocity (which is always called when dry deposition is on) need Monthly LAI either from LAI-streams or from the surface dataset to determine the seasonal_index which is used in drydepvelocity parameterization. The LAI that is read from surface is actually LAI for a reference year 2005 and does not change at all. (check depvel_compute routine).

Since fates does not actually read any PFT data from CTSM, you will get a dimension mismatch error, since surface dataset has a 78/16 pft dimension length, while sets pft dimension to whatever.

@mvertens
Copy link

@kjetilaas - I have created a new discussion #78 that summarizes how drydep velocities are computed in CTSM. Hopefully this will provide some clarification.

@kjetilaas kjetilaas changed the title Cannot run FATES-NOCOMP in coupled model Drydep issue with FATES-NOCOMP in coupled model Aug 23, 2024
@rosiealice
Copy link
Collaborator

In the meeting today we had a look at this code and determined that the dry dep needs to know from the vegetation model

  1. The Wesely (1989) plant funcitonal type (currently hard-wired from the CLM PFT space in the code)
  2. The season index, used to 'calculate' the surface resistance, thus used in the drydep routine.

A reasonable first step to fixing this is to create a wesely PFT index varialbe in the FATES parameter file, and send that to the dry dep code. This is a very similar approach to what we did in MEGAN and (compared to the alternative of going via the HLM PFT definition) is robust to changes in the HLM PFT definition that FATES cannot control. (n.b. the ELM will change to having more arctic shrub PFTs in the not too distant future. So we need the FATES parameter file to be somewhat independant of that).

So what I plan to do as a first step (before tckling the season index) is to make this PFT index.

But my question is which codebase should I base it off? The main FATES repo? The tag we are using in NorESM now from https://github.com/NorESMhub/CTSM/blob/noresm/Externals_CLM.cfg ?

Of course we will want to get this back into the FATES code in the not too distant future (before it becomes stale). So I don't want to start off in the wrong place. @mvertens @mvdebolskiy do you have any thoughts?

@mvdebolskiy
Copy link
Collaborator

You can start from sci.1.73.0_api.35.0.0 tag on NGEET/fates which is checked out by ctsm5.2.005-noresm_v3, the latest version of CTSM we have on NorESMHub right now.

@rosiealice
Copy link
Collaborator

OK great. I will do that. :)

@kjetilaas kjetilaas added this to the NorESM2.5 milestone Sep 13, 2024
@kjetilaas kjetilaas moved this from Todo to In Progress in NorESM Development Sep 13, 2024
@rosiealice
Copy link
Collaborator

Say for the sake of argument I had made some FATES code modification in a branch here:
https://github.com/rosiealice/fates/tree/drydeposition_sci.1.75.0_api.35.0.0

and these was based off of a tag: sci.1.75.0_api.35.0.0 (I used this one as it contains the vertical respiration scaling we need for the NOCOMP simulations, but does not change the API. NorESM currently points to sci.1.73.0_api.35.0.0)

And the changes relative to that tag were:
https://github.com/NGEET/fates/compare/sci.1.75.0_api.35.0.0...rosiealice:fates:drydeposition_sci.1.75.0_api.35.0.0?expand=1

Where should I make a pull request for this code? The current version of FATES on NorESMhub is a long way behind this tag... Should we updated it before I try and do a PR?

@rosiealice
Copy link
Collaborator

...similarly, should I start the modifications for the parallel CTSM side code from here? Or?

https://github.com/NorESMhub/CTSM/tree/noresm

@kjetilaas
Copy link
Collaborator Author

I would say we should update both, and make the PR to the NorESMhub versions of FATES and CTSM.

@mvertens
Copy link

I agree with @kjetilaas.

@mvdebolskiy
Copy link
Collaborator

I've made NorESMhub/noresm branch. You can make your pull request into that one. It's on the sci.1.75.0_api.35.0.0 tag right now

@rosiealice
Copy link
Collaborator

Brill. Thanks...

@rosiealice
Copy link
Collaborator

So the issue I discovered during the meeting was indeed the last edge-case I needed to deal with, and the code is now running in coupled NOCOMP mode.

Can someone make a 'noresm' branch on the NorESMhub/fates repo so I can push the changes there? @mvertens @mvdebolskiy

Ignore this branch that is already there
https://github.com/NGEET/fates/commits/drydeposition_sci.1.75.0_api.35.0.0/
this is not up to date.

@mvertens
Copy link

@rosiealice - there already is a noresm branch for the fates repo on noresmhub.
If you do the following in your code sandbox in the clm/src/fates directory:

> git remote add noresmhub https://github.com/NorESMhub/fates
> git fetch noresmhub
> git checkout noresm

You will see the following (if you use my git logg command)
$ git logg

*   6c85a1b0 (HEAD -> noresm, tag: sci.1.75.0_api.35.0.0, noresmhub/noresm) - (6 months ago) Merge pull request #1161 from jenniferholm/calibration_dayl-factor-switch - Ryan Knox
|\
| * a9b52307 - (6 months ago) reverting to parameter file from main, because daylength factor switch was already in there - Ryan Knox
| *   668fabd4 - (6 months ago) trivial merge resolution - Ryan Knox
| |\
| |/
|/|
* |   04afb122 (tag: sci.1.74.0_api.35.0.0) - (6 months ago) Merge pull request #1149 from JessicaNeedham/jfn-leafmr-vert-scaling - Ryan Knox

You want to do a PR to noresm in the noresmhub repository with your code. I'm happy to have a zoom chat early this afternoon if that would help.

@mvertens
Copy link

Best practice is that branches should not be 'pushed' changes back to noresmhub repositories. The suggested approach is to always do PRs that have at least one review. In our case this implies making PRs to the noresm fates branch in noresmhub.

@rosiealice
Copy link
Collaborator

Sorry, I meant pull of course :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

No branches or pull requests

4 participants