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

Balance checks not turned on for CLM-DART simulations #2525

Open
braczka opened this issue May 4, 2024 · 4 comments
Open

Balance checks not turned on for CLM-DART simulations #2525

braczka opened this issue May 4, 2024 · 4 comments
Labels
type: bug something is working incorrectly

Comments

@braczka
Copy link

braczka commented May 4, 2024

Brief summary of bug

When running CLM-DART (multi-instance run w/ DATA_ASSIMILATION_LND=TRUE) the DA_nstep variable within BalanceCheckMod.F90 and CNVegetationFacade.F90 does not appear to get updated. This prevents the carbon, nitrogen, water and energy balance checks from being turned on after skip_steps is passed.

General bug information

Version: release-cesm2.2.03

The bug prevents balance checks from being performed when DA is implemented and for all timesteps thereafter.

Details of bug

See summary above. We have usually circumvented this issue by utilizing the is_first_restart_step variable
initialized within clm_time_manager.F90 to indicate the first time step after DA. This approach is suboptimal, and
doesn't seem to leverage intent of code.

Important details of your setup / configuration so we can reproduce the bug

Compset: 2000_DATM%GSWP3v1_CLM50%BGC-CROP_SICE_SOCN_MOSART_SGLC_SWAV
Resolution: f09_f09_mg17
Machine: Derecho
Compiler: Intel

For example of specific configuration details see:
case directory: /glade/work/bmraczka/cases/ctsm_cesm2.2.03/clm5_print_e5
run directory: /glade/derecho/scratch/bmraczka/ctsm_cesm2.2.03/clm5_print_e5/run

Important output or errors that show the problem

No errors will be output, although it should trigger warning message:
--WARNING-- skipping CN balance check for first timesteps after startup or data assimilation

@ekluzek ekluzek added tag: next this issue should get some attention in the next week or two type: bug something is working incorrectly labels May 4, 2024
@ekluzek
Copy link
Contributor

ekluzek commented May 4, 2024

@braczka with cesm3.0 coming, the cesm2.2 release doesn't have much of a future. So I'm loathe to fix this in cesm2.2, unless it's vital for some reason. I think you might just be reporting on where you see the error, and want us to fix it in the latest which is what we want to do. It's always important to give the actual version you see the problem in.

The thing we are very interested in though is to have this working in the latest development version of CTSM. So will fix it there.

From looking at the latest code, I think this is a problem with NUOPC and probably works correctly with MCT (MCT has the correct call to update_DA_nstep, but it's missing in NUOPC). So first, are you running with NUOPC or MCT (xml variable COMP_INTERFACE)? If NUOPC, try running with MCT and see if it then works.

@ekluzek
Copy link
Contributor

ekluzek commented May 4, 2024

@braczka this also makes me want to make sure that our testing for DART functionality is sufficient. And that would need the DART team to work with us a bit to make sure we have appropriate coverage. I don't think we should do that immediately, but we do want to make sure CTSM in CESM3 is good for working with DART.

For this problem, the solution I see is to add some error checking in the clm_time_manager to make sure that update_DA_nstep is called. And that's an easy thing to do. We can also add this to our unit-testing which should help.

@braczka
Copy link
Author

braczka commented May 6, 2024

Thank you @ekluzek. I am fairly certain it is using MCT in my example case. I am using an older tag (release-cesm2.2.03), but after checking the regions of the code involving DA_nstep (view git blame), it looks like there have been no recent commits addressing this part of the code, so I think the problem still exists in the most recent version.

This may be an easy fix where update_DA_nstep or some other subroutine is not properly called. It seems that any fix applied to the most recent CTSM version will also work for the older tag we are using with DART. I am not very familiar with this part of the code, so may be something we can solve easily with a bit of guidance. We currently have a workaround patch involving is_first_restart_step, but definitely suboptimal.

@ekluzek ekluzek removed the tag: next this issue should get some attention in the next week or two label May 23, 2024
@ekluzek
Copy link
Contributor

ekluzek commented May 23, 2024

I chatted with @braczka after the CTSM science meeting about this. He notes that we should also work with @hkershaw-brown in DART as the SE we should interact with.

I think there are actually two bugs here, one for MCT and another for NUOPC. Neither of these need to be fixed by the CESM3 science capability freeze end of June, but we do want this in place for the CESM3.0 release so users can use DART with CTSM without requiring a code modification.

One of the things I want to do, is to try to get our testing setup so that we don't have problems running CTSM with DART. I think there are some unit-testing things that I can do there. Making sure that the update isn't called each time-step. But we also need to look at @braczka case above so that we know how CTSM is being used in DART and we have a test case that's similar to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug something is working incorrectly
Projects
None yet
Development

No branches or pull requests

2 participants