ufs-community / UFS_UTILS

Utilities for the NCEP models.
Other
21 stars 107 forks source link

Create unit tests for ocean_merge code #944

Open GeorgeGayno-NOAA opened 4 months ago

GeorgeGayno-NOAA commented 4 months ago

Follow the procedures used in other programs:

Unit tests should be created for all major functions of the code:

HenryRWinterbottom commented 4 months ago

@GeorgeGayno-NOAA My branch for this issue is #944.

Is there a plan to add the respective test files to the FTP site for the ocean_merge application unit-tests?

GeorgeGayno-NOAA commented 4 months ago

@GeorgeGayno-NOAA My branch for this issue is #944.

Is there a plan to add the respective test files to the FTP site for the ocean_merge application unit-tests?

If your tests require large binary input data, we can host it here: https://ftp.emc.ncep.noaa.gov/static_files/public/UFS/ufs_utils/unit_tests/

Small ASCII data files can be stored in Github. For example: https://github.com/ufs-community/UFS_UTILS/tree/develop/tests/chgres_cube/data

HenryRWinterbottom commented 4 months ago

@GeorgeGayno-NOAA I have finished a first cut of the ocean_merge refactor. Please see here.

I implemented the ncio package. This is a NCEP-libs supported package but will require a spack-stack update. Further, some additions to the CMake application are also required. If/when you are OK with the refactoring, I can update the CMake accordingly and finish up the unit-tests.

Please let me know how you'd like to proceed.

GeorgeGayno-NOAA commented 4 months ago

@GeorgeGayno-NOAA I have finished a first cut of the ocean_merge refactor. Please see here.

I implemented the ncio package. This is a NCEP-libs supported package but will require a spack-stack update. Further, some additions to the CMake application are also required. If/when you are OK with the refactoring, I can update the CMake accordingly and finish up the unit-tests.

Please let me know how you'd like to proceed.

This looks fine.

What spack-stack update is required?

HenryRWinterbottom commented 4 months ago

@GeorgeGayno-NOAA We will need to make sure ncio is included in the stack. I can make that PR, I just wanted to make sure we were on the same page before diving in.

GeorgeGayno-NOAA commented 4 months ago

@GeorgeGayno-NOAA We will need to make sure ncio is included in the stack. I can make that PR, I just wanted to make sure we were on the same page before diving in.

I checked Hercules and Hera. ncio was part of the stack. But I did not find ncio on WCOSS2. Getting libraries installed on WCOSS2 is not a quick or simple process. So, I would move forward with the assumption that ncio is not available.

HenryRWinterbottom commented 4 months ago

@GeorgeGayno-NOAA OK, thank you for checking into that.

I will need to rewrite some code, but it shouldn't be too terrible. I will follow-up here once I do.

Thanks again.

HenryRWinterbottom commented 4 months ago

@GeorgeGayno-NOAA I made appropriate updates, including to the relevant CMakeList.txt.

This simplifies things a bit, and as discussed, removes the ncio dependency thus saving a PR into spack-stack and provides WCOSS compatibility.

Please review. Once you are satisfied, I can finish up the respective unit-tests.

GeorgeGayno-NOAA commented 4 months ago

@GeorgeGayno-NOAA I made appropriate updates, including to the relevant CMakeList.txt.

This simplifies things a bit, and as discussed, removes the ncio dependency thus saving a PR into spack-stack and provides WCOSS compatibility.

Please review. Once you are satisfied, I can finish up the respective unit-tests.

This looks fine.

GeorgeGayno-NOAA commented 4 months ago

@HenryWinterbottom-NOAA - please cease work on this issue for now.