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

NUOPC Cap Improvements #1645

Merged
merged 3 commits into from
Jan 31, 2025
Merged

NUOPC Cap Improvements #1645

merged 3 commits into from
Jan 31, 2025

Conversation

danrosen25
Copy link
Contributor

Description

This pull requests maintains the cap by cleaning up the code for better maintainability and enhancements. Enhancements were made to the documentation, the import and export state initialization, and the import state checking

Resolves #1644

Testcase

Uses NASA Land Coupler for testing
https://github.com/esmf-org/NASA-Land-Coupler/tree/feature/lisf_upgrade

bit-for-bit results for coupled tuolumne cases

SUCCESS: coupled_tuolumne.ens020.noahmp_v4.0.1_da.nldas2 LIS_HIST_201403020000.d01.nc
SUCCESS: coupled_tuolumne.ens020.noahmp_v4.0.1_da.nldas2 LIS_HIST_201403030000.d01.nc
SUCCESS: coupled_tuolumne.noahmp.nldas2 LIS_HIST_201510020000.d01.nc
SUCCESS: coupled_tuolumne.noahmp.nldas2 LIS_HIST_201510030000.d01.nc

* add README file and guides
* update documentation using Doxygen
* add gnu/intel debug options for BUILD_TYPE=DEBUG
* update missing field value to 9.99e20
* add initialization options for import and export states
* add missing value check (disable with MISSINGVAL_SKPCPY or MISSINGVAL_IGNORE)
* call lisfinalize during cap finalize
* move configuration options to LIS_NUOPC_Flags.F90
@danrosen25
Copy link
Contributor Author

danrosen25 commented Nov 26, 2024

@jvgeiger @dmocko @emkemp
Hi all,
I'm submitting updates to the NUOPC coupling interface (NUOPC Cap). It's going to come in a few separate different pull requests to isolate the changes. This first change is NUOPC cap only changes. I ran testing in a coupled configuration but it will need some offline regression tests.

@danrosen25
Copy link
Contributor Author

@sujayvkumar
I'm including Sujay in this conversation.

@dmocko dmocko added Documentation Update to documentation (User's Guide or within the code) enhancement New feature or request Cleanup 🧹 labels Dec 30, 2024
@jvgeiger
Copy link
Contributor

Hello Dan. I have one fix to address compiling with GNU compilers. In LIS_NUOPC_Cap.F90, change all

specialStringList=(/"min","max","bit16","maxplus"/), &

to

specialStringList=(/"min    ","max    ","bit16  ","maxplus"/), &

The GNU compilers want all string literals in an array constructor to have the same length.

I am unable to push this fix.

@danrosen25
Copy link
Contributor Author

@jvgeiger
Did you see anymore GNU errors? I'll have to circle back to set up an environment for building with GNU
You can build the cap code by

> cd LISF/lis
> ./configure
> cd runmodes/nuopc_cpl_mode
> make nuopcinstall

This was a reminder that I want to clean up the cap build eventually too.

@jvgeiger
Copy link
Contributor

Thanks. It compiles without errors.

@jvgeiger
Copy link
Contributor

By the way, if you modify your runmodes/nuopc_cpl_mode/Makefile to say

( cd $(MODEL_DIR); ./compile -j 16 )

LIS will compile faster. I am not suggesting this for this pull request. Just for your information.

@danrosen25
Copy link
Contributor Author

@jvgeiger
Thanks! Years ago when I NUOPC caps I would wrap the model build inside the cap build but I've since started to add build flags to the native build system. So it might be better to add an option to the ./compile script. I see you're using getopts, which only supports short flags, so what flag makes sense?

@jvgeiger
Copy link
Contributor

Hello. LIS' compile script is just a simple wrapper for executing make that covers the most common use-case, which is compiling a LIS executable. LIS' Makefile can also has three other targets for compiling LIS as a library for coupling with others models like NU-WRF. In those cases, we skip the compile script and run make from the command line. So having your own build steps for NUOPC Cap is ok.

Maybe you can simplify your Makefile by using LIS' make lislib target instead of calling LIS' compile script. The make lislib target will create LISF/lis/make/lislib.a from all LIS' OBJS, similarly to your liblis.a. (Unfortunately, I just found a bug in this target. I will fix it soon.)

To natively add NUOPC Cap support into LIS' Makefile, we will need a new target, of course, but that is no problem.

Now, usually, to add new source files into LIS' Makefile, you add an entry into the default.cfg file. But, then the NUOPC Cap source files will be compiled with the same compiler flags as the rest of the LIS source files. I see in your Makefile that you use different compiler flags. If this is required, then you must add explicit rules for those files instead of adding an entry into the default.cfg file.

Note: For NUOPC Cap, its entry in the default.cfg file should default to "enabled: False".

I will accept this pull request.

Changing the build scripts should go into a new pull request.

@jvgeiger jvgeiger merged commit a47ac50 into NASA-LIS:master Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup 🧹 Documentation Update to documentation (User's Guide or within the code) enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cleanup NUOPC Cap for LIS
3 participants