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

Have CTSM give a better error message when ndep files are missing especially for the case when coupled to CAM #946

Closed
ekluzek opened this issue Mar 23, 2020 · 11 comments · Fixed by #1079
Assignees
Labels
type: enhancement new capability or improved behavior of existing capability
Milestone

Comments

@ekluzek
Copy link
Contributor

ekluzek commented Mar 23, 2020

Brief summary of bug

Dummy ndep files are required even for WACCM simulations that don't need them.

General bug information

CTSM version you are using: release-clm5.0.30
Does this bug cause significantly incorrect results in the model's science? No

Configurations affected: Compsets with CTSM and WACCM

Details of bug

If a WACCM compset doesn't find a ndep file, the case will fail to build, or run preview_namelists. This is even though when WACCM is being run it doesn't actually
use the ndep file. It would be better for documentation to NOT list a file that isn't actually being used for the case. When run with WACCM the nitrogen-deposition is coming from CAM.

The code can check for this by making sure it either gets n-dep from CAM or from the ndep file. If it doesn't get it from either it should fail.

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

The case that failed was a SSP5-3.4 case (since we don't have a ndep file for it). It would also fail for the non Tier-I SSP scenarios where we don't have ndep files. And note for I compsets it will fail when it can't find appropriate presaero files.

It would also be helpful to make the error from build-namelist to be more helpful.

Important output or errors that show the problem

ERROR: Command /glade/u/home/cmip6/cesm_tags/release-cesm2.1.2/components/clm/bld/build-namelist failed rc=255
out=CLM adding use_case 1850-2100_SSP5-3.4_transient defaults for var 'sim_year' with val '1850'
CLM adding use_case 1850-2100_SSP5-3.4_transient defaults for var 'sim_year_range' with val '1850-2100'
CLM adding use_case 1850-2100_SSP5-3.4_transient defaults for var 'ssp_rcp' with val 'SSP5-3.4'
CLM adding use_case 1850-2100_SSP5-3.4_transient defaults for var 'use_case_desc' with val 'Simulate transient land-use, aerosol deposition, and Nitrogen deposition changes from 1850 to current day with historical data, and then to 2100 with the CMIP6 SSP5-3.4 scenario'
err=ERROR : CLM build-namelist::CLMBuildNamelist::add_default() : No default value found for stream_fldfilename_ndep.
Are defaults provided for this resolution and land mask?

@ekluzek ekluzek added type: enhancement new capability or improved behavior of existing capability branch tag: release Changes go on release branch as well as master labels Mar 23, 2020
@ekluzek ekluzek added this to the cesm2.1.4 milestone Mar 23, 2020
@ekluzek ekluzek self-assigned this Mar 23, 2020
@ekluzek
Copy link
Contributor Author

ekluzek commented Mar 23, 2020

@negin513 and @billsacks can you comment on this request? The one downside on this is that I have to check the compset to see if WACCM is on in CAM. This means I need to know about components outside of CTSM. In general we try to build CTSM such that it doesn't know (nor care) about what components it's connected to. Doing this breaks that. So even though I think it's a good idea in this case -- it breaks a convention that's also a good idea.

@billsacks
Copy link
Member

@ekluzek I share your mixed feelings here. Can you say more about how you intend to do this checking? We shouldn't have checks of the compset inside CLMBuildNamelist, because that now needs to work for LILAC, too (where we don't have compsets); it would be more acceptable for there to be some logic in cime_config/bldnml that sets a flag to CLMBuildNamelist (like '--atm-provides-ndep') (note that LILAC will use its own version of bldnml).

I'd (strongly) prefer a solution that put more of the burden on CAM, so that they maintain all of the logic that says whether they are going to provide ndep fields. I imagine having CAM add a new xml variable in their cime_config/config_component.xml like ATM_PROVIDES_NDEP. Then they could set this based on the compset. Then, if the logic for this variable needs to change in the future, they would maintain it themselves; it seems much more likely to be kept correct this way than if the logic is in CTSM. Note that datm would also need to have this variable added to its cime_config/config_component.xml, set appropriately based on whether datm provides this field (so I guess it would always be false for datm now, but that could change in the future if this capability were added to datm).

It feels like the long-term correct solution here is to require consistency in terms of where this field comes from: If CAM provides this field in some situations, then it should be responsible for ALWAYS providing it; CAM should be the one who maintains the data on ndep if it is not able to provide it prognostically. Similarly, datm would provide these data when running with datm. But I realize that this is a larger change than is probably possible right now.

@ekluzek
Copy link
Contributor Author

ekluzek commented Mar 23, 2020

Thanks for pointing out that this would likely cause a problem for LILAC. I hadn't thought about that. I think we'll need to talk with some of the CAM people about this. I'll add it as a topic for tomorrows CSEG meeting. And we'll probably need to schedule another meeting with CAM folks as well.

It would make sense to make CAM/datm always provide ndep. That would actually remove the problem of having to coordinate the ndep datasets between OCN and LND, like we have to do now. And for both OCN and LND to have to save and point to the same data. But, like you say that's probably a larger change than we want to do now.

@ekluzek
Copy link
Contributor Author

ekluzek commented Mar 24, 2020

The solution I have for now is to improve the error code, and make it a warning, so there's a way to go around it. So here's the error I've put in now...

missing_ndep_file
Warning : CLM build-namelist::CLMBuildNamelist::setup_logic_nitrogen_deposition() : Did NOT find the Nitrogen-deposition forcing file (stream_fldfilename_ndep) for this ssp_rcp
One way to get around this is to point to a file for another existing ssp_rcp in your user_nl_clm file.
If you are running with CAM and WACCM chemistry Nitrogen deposition will come through the coupler.
This file won't be used, so it doesn't matter what it points to -- but it's required to point to something.

 -- Add -ignore_warnings option to CLM_BLDNML_OPTS to ignore this warning

@wwieder
Copy link
Contributor

wwieder commented Mar 24, 2020 via email

@ekluzek ekluzek changed the title Have CTSM check the compset and if WACCM is being run to NOT require ndep files Have CTSM give a better error message when ndep files are missing especially for the case when coupled to CAM Mar 24, 2020
@ekluzek
Copy link
Contributor Author

ekluzek commented Mar 24, 2020

We talked about this at the CSEG meeting and I started an issue in CAM. I'm making this issue to just deal with the short term issue of a better error message. If it looks like CAM is going to do something longer term that we need to connect with I'll open an issue that dovetails into it.

Here's the CAM issue that's been opened.

ESCOMP/CAM#104

@ekluzek
Copy link
Contributor Author

ekluzek commented Mar 25, 2020

We've gone around and around on the CAM issue. The long term solution is to have CAM always send ndep either as calculated or from files. We don't see a short term solution at all.

CAM user's can also easily turn off sending of ndep even when it could be sent (which just means that CTSM will use it's own ndep file). So I think that's OK. But, it goes back to saying that we can't easily detect that ndep will be sent from CAM (a compset that would send it normally, could be changed to not send it).

The one thing that we possibly should do though is to check if ndep being sent from the CPL is identically zero -- if so we should abort. As I said It turns out it's pretty easy for a CAM user to do that. I assume ndep could be small, but would never be zero, and that using zero'ed ndep would result in bad simulations.

The other suggestion is if an appropriate ndep file can't be found -- it sets it to a dummy file name (something like do_not_read_this_ndep_file.nc) and if the model trys to read it -- it will abort on initialization at runtime. I think that's bad because it has the model aborting at run-time rather than at preview_namelist time.

@wwieder and others what do you think about those ideas (aborting if ndep from CPL is zero, and setting a dummy file that would never be read)?

@wwieder
Copy link
Contributor

wwieder commented Mar 25, 2020 via email

@ekluzek
Copy link
Contributor Author

ekluzek commented Mar 25, 2020

Yes, I'll add it to the schedule.

@ekluzek
Copy link
Contributor Author

ekluzek commented Mar 26, 2020

In our meeting this morning that included: @wwieder @dlawrenncar @billsacks, @negin513 and myself we decided that the improvement on the error in preview_namelist is sufficient. So we won't do other proposals including checking for identically zero, nor setting ndep when coupled to CAM to a special filename -- that triggers an abort if it is actually attempted to be read.

@ekluzek
Copy link
Contributor Author

ekluzek commented Mar 27, 2020

One suggestion that @billsacks has was to make sure the warning points out that if you are coupled to CAM and it's sending ndep (i.e. because it's a WACCM compset) that the warning won't matter. In all other cases the model will die at runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement new capability or improved behavior of existing capability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants