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

Limits the definition of cohort dim in ELM restart file #5699

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

bishtgautam
Copy link
Contributor

@bishtgautam bishtgautam commented May 18, 2023

In ELM restart file, the cohort dimension is only defined when FATES is turned on.
This code modification allows ELM to write elm.rh0 when hist_empty_htapes=.true.
is set in user_nl_elm.

Fixes #5694
[BFB]

@bishtgautam bishtgautam requested a review from glemieux May 18, 2023 03:18
@bishtgautam bishtgautam added Land bug fix PR non-BFB PR makes roundoff changes to answers. labels May 18, 2023
@bishtgautam
Copy link
Contributor Author

bishtgautam commented May 18, 2023

This PR removes the dimension (cohort) from ELM restart file, except when FATES is turned on. Thus, cprnc will report DIFFs for many files.

In ELM restart file, the `cohort` dimension is only defined
when FATES is turned on.

Fixes #5694
[non-BFB]
@bishtgautam bishtgautam changed the title Limits the definition of cohort dim in ELM nc files Limits the definition of cohort dim in ELM restart file May 18, 2023
@bishtgautam
Copy link
Contributor Author

@glemieux @rgknox For elm.h0, all the dimension names for FATES have a prefix fates_. For elm.r*, do you want to stick with cohort as the dim name or should it be changed to fates_corhort or something else?

@rljacob
Copy link
Member

rljacob commented May 18, 2023

But will this change the elm.h0 files? The tests ignore the restart files.

@rgknox
Copy link
Contributor

rgknox commented May 18, 2023

That dimension in the restart file is maintained to handle information at the "cohort" level, but it also holds other data structures that have information density higher than the column level. So.. its not exclusive to cohort stuff. I suppose you could argue that its more of a fates utility dimension.

@peterdschwartz
Copy link
Contributor

@bishtgautam running the tests with this merged to next and didn't encounter any DIFFs so this can be changed to BFB I think.

@bishtgautam bishtgautam added BFB PR leaves answers BFB and removed non-BFB PR makes roundoff changes to answers. labels Jun 22, 2023
@bishtgautam
Copy link
Contributor Author

@peterdschwartz I have updated the description to be BFB.

@glemieux
Copy link
Contributor

glemieux commented Jun 26, 2023

@bishtgautam running the tests with this merged to next and didn't encounter any DIFFs so this can be changed to BFB I think.

@bishtgautam @peterdschwartz do you want me to create a PR to this pointing to the latest fates tag and updating the paramfile pointer after NGEET/fates#1041 is integrated? Or should I simply create a new, separate PR since this PR has been tested already? As a reminder, the fates PR fixes the DIFFs that cropped up with #5599 (comment).

@rljacob
Copy link
Member

rljacob commented Jul 6, 2023

status: @glemieux and @bishtgautam will talk.

@glemieux
Copy link
Contributor

glemieux commented Jul 6, 2023

Per our conversation, this PR can go forward. I'll be creating a seperate PR to update to the new fates tag and reinstate some fates debug tests to the land developer tests.

peterdschwartz added a commit that referenced this pull request Jul 16, 2023
In ELM restart file, the cohort dimension is only defined when FATES is turned on.
This code modification allows ELM to write elm.rh0 when hist_empty_htapes=.true.
is set in user_nl_elm.

Fixes #5694
[BFB]
@peterdschwartz peterdschwartz merged commit 0e8ca45 into master Jul 17, 2023
@peterdschwartz peterdschwartz deleted the bishtgautam/lnd/restart branch July 17, 2023 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB bug fix PR Land
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow ELM to write elm.rh0 when hist_empty_htapes=.true.
6 participants