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

adding lai_dtlimit to namelist #8

Merged
merged 10 commits into from
Apr 16, 2024
Merged

Conversation

wwieder
Copy link
Collaborator

@wwieder wwieder commented Apr 13, 2024

Description of changes

Allow dtlimit to be set for the LAI streams

Specific notes

Contributors other than yourself, if any:

CTSM Issues Fixed (include github issue #):

Are answers expected to change (and if so in what way)? Only potentially if used.

Any User Interface Changes (namelist or namelist defaults changes)?
New lai_dtlimit namelist item.

Testing performed, if any:

Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @wwieder nice work here!

There's one critical change that has to happen and that's to add the shr_mpi_bcast call for lai_dtlimit. And this was missing for lai_mapalgo so you should also add it for it as well.

The other things I have are suggestions. One is suggestion about more text for the description in the namelist_definition file. Another is to add lai_dtlimit and lai_mapalgo to the log output. The others are linked together and it's for setting the default value in the FORTRAN code rather than the namelist.

The reason I suggest setting the default in the FORTRAN code rather than in the namelist, iis because this is likely something that will be rarely changed by users. So it's nice to hide it. If you add the default in the namelist defaults file, it will ALWAYS show up in the lnd_in file. This is useful to do for things that you want to remind users that they are available -- or for items that are often changed for different setups.

@@ -144,7 +146,7 @@ subroutine lai_init(bounds)
stream_yearAlign = model_year_align_lai, &
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above this there needs to be a shr_mpi_bcast call for lai_dtlimit as well (and it looks like this is missing for lai_mapalgo). I also suggest adding to the logging write statements as well (and same for lai_mapalgo).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ekluzek I think I've done this correctly, notably the critical part to "add shr_mpi_bcast call for lai_dtlimit.", but can you please confirm?

If this look OK, I think we can merge this PR to @olyson's PR and maybe @slevis-lmwg work to bring this to b4b-dev? Having more functional usermods would be helpful for @TeaganKing to make progress on the larger PLUMBER project.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one change here, lai_dtlimit is a real rather than character, so remove the trim() function call around it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything else looks great. So I'll mark as good to go...

bld/namelist_files/namelist_definition_ctsm.xml Outdated Show resolved Hide resolved
bld/CLMBuildNamelist.pm Outdated Show resolved Hide resolved
bld/namelist_files/namelist_defaults_ctsm.xml Outdated Show resolved Hide resolved
src/cpl/share_esmf/laiStreamMod.F90 Outdated Show resolved Hide resolved
@wwieder
Copy link
Collaborator Author

wwieder commented Apr 14, 2024

Thanks for your suggestions, @ekluzek. I'll get to those tomorrow.

Here I'm posting changes to usermods that are a bit cleaner and seem to work for spinup of SP and BGC cases. Transient cases also run as expected, but fail with this error associated with cycling back to the start of the datm file

forrtl: severe (61): format/variable-type mismatch, unit -131, file /glade/u/home/wwieder/scratch/PLUMBER2_tests/ZM-Mon_hist_BGC_test1/run/atm.log.4116681.desched1.240414-083718

@olyson
Copy link
Owner

olyson commented Apr 14, 2024

I was looking over the differences between your BGC/SP hist cases and my own. One thing I see is that you are running NO_LEAP, while I ran GREGORIAN. The datm forcing and lai streams both have data for 2008 leap day. I'm not sure if that has to do with the errors you are seeing though

@olyson
Copy link
Owner

olyson commented Apr 15, 2024

Another thing to try is to set taxmode to 'extend'. My case seems to come of the box with that setting. The forcing at the end of the run is unique (the last two timesteps don't use the same forcing), but maybe it needs to read the the last time sample in the file one more time even though it doesn't use the data.
I was able to rerun my case using release-clm5.0.37, which has been ported to derecho. It runs fine with taxmode set to extend, but fails if I set it to 'cycle'.
On the other hand, the taxmode for the lai streams is 'cycle' and I haven't changed it. But I do have dtlimit for the lai streams set very high (1.e30; following some other example I had). So you could try changing that also.

@wwieder
Copy link
Collaborator Author

wwieder commented Apr 15, 2024

ah, @olyson this is brilliant! I've add this to the PR so that transient runs (using a HIST compset) are set with taxmode=extend and dtlimit=1e30 (required for some reason), but spinup (using an I2000 compset are cycle and 30, respectively).

Now (I think) we can run both spinup and historical cases using usermods that are customized for each site!

One additional notes here, I had to set lai_dtlimit=1e30 for the transient SP cases too.

I'm going to do a few more tests, but I think this is looking good!

@wwieder
Copy link
Collaborator Author

wwieder commented Apr 15, 2024

@slevis-lmwg I realize that merging this part of the PR onto the CTSM b4b-dev or master will require additional testing, as aspects of the code have changed with addition of dtlimit to lai_streams

@ekluzek ekluzek added the enhancement New feature or request label Apr 15, 2024
@slevis-lmwg
Copy link
Collaborator

@ekluzek @wwieder
I see at the top of the page that merging this PR will merge to olyson:PLUMBERcsv

Is this the intent, or does this PR need rebasing to b4b-dev?

@wwieder wwieder merged commit 4a19a1c into olyson:PLUMBERcsv Apr 16, 2024
2 checks passed
@slevis-lmwg slevis-lmwg deleted the PLUMBERcsv branch April 16, 2024 15:23
olyson pushed a commit that referenced this pull request Sep 17, 2024
Bring landuse api up to date with `branch_tags/tmp-240620.n02.ctsm5.2.007`
olyson pushed a commit that referenced this pull request Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants