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

Can this repo be built stand-alone? #94

Closed
edwardhartnett opened this issue Apr 2, 2020 · 9 comments
Closed

Can this repo be built stand-alone? #94

edwardhartnett opened this issue Apr 2, 2020 · 9 comments

Comments

@edwardhartnett
Copy link
Collaborator

Can this repo be built stand-alone?

That will be required for CI to be set up with Travis.

CMake has the capability to build this as part of ufs_weather_model, and also in stand-alone mode.

@edwardhartnett
Copy link
Collaborator Author

@junwang-noaa as mentioned in this old issue, and discussed at the meeting yesterday, it will be necessary to build this repo in stand-alone mode to enable testing.

It's OK if this requires FMS or other libraries like netCDF, but it should not require UFS in order to test fv3atm.

@edwardhartnett
Copy link
Collaborator Author

I would like to work on this, but I am elbow-deep in NCEPLIBS and will be for some time.

@junwang-noaa I highly recommend that this issue be resolved so that fv3atm can be built and tested in stand-alone mode. Then I would encourage all programmers to take a few tests sprints, in which everyone put aside new work for a bit, and wrote unit tests for existing code.

@aerorahul
Copy link
Contributor

@edwardhartnett

This repo can be built as a standalone library -- yes. We already do that. But what will you do with just a library?
The fv3dycore (part of this repo) is linked as a library, as are other components ccpp, gfsphysics, etc.
The fv3dycore can be built as a standalone executable e.g. fv3dycore.x using atmos_model.F90 as the driver. But what would you test with the standalone dycore driver? GFDL does testing of the dycore extensively and with this (and other coupled) drivers.

To drive the "fv3atm" component as a standalone executable e.g. fv3atm.x is essentially what the UFS weather model is.
This already allows for testing the physics suites. I agree these are not unit tests.

I am a bit unclear as to what you wish to accomplish by building this as repo as a standalone "thing". To add tests, I would imagine a lot of tiny programs that test the functionality of individual subroutines in this library (or even subcomponent libraries since fv3atm is a collection of its components) by providing inputs (as simulated by the whole big UFS) and comparing them with baseline expected outputs.
I would imagine you could use this standalone driver as the starting point for writing the individual tests to setup FMS, MPP, domain decomposition etc. Is that your intention?

@edwardhartnett
Copy link
Collaborator Author

@aerorahul yes, you have summarized a great plan for getting unit testing into this code.

Last time I tried to add a unit test, the repo would not build for me, which is why I added this issue. From your post I understand that this has changed? Good! If we can get one test running we can close this issue. Then, next time I want to write a test for some of the fv3atm I/O code, it will be possible.

Developing software without unit tests has been proven to be more expensive, more error prone, and more frustrating for programmers. This is well-understood software engineering. There is no reason to believe that this repo is the exception to this commonplace engineering truism.

All over DC, Boulder, and any other tech hub, programmers are writing unit tests for their code as they write e-commerce systems, actuarial calculations for insurance companies, embedded software in cars, toys, and phones, and most importantly, video games for teenagers. They all do this testing because it is the cheap and reliable way to develop software that always works.

Do you agree with me that fv3atm is at least as important as "Call of Duty 3"?

If so, then let's get started on some unit testing.

@aerorahul
Copy link
Contributor

@edwardhartnett Don't look at me.

@edwardhartnett
Copy link
Collaborator Author

edwardhartnett commented Oct 9, 2020

image

@edwardhartnett
Copy link
Collaborator Author

Apparently this repo still cannot be built stand-alone.

The build systems have painted this repo into a corner where we can't add testing or documentation. How can we fix this?

@arunchawla-NOAA
Copy link

@edwardhartnett @aerorahul @junwang-noaa @climbfuji

the PRs related to this issue are either in draft mode or closed. Is this issue still open ?

@edwardhartnett
Copy link
Collaborator Author

Certainly this repo should be able to build and test itself.

I have closed my open PR, but I encourage leaders of the FV3 team to bring their CMake build up to date, including stand alone builds, checking for dependencies and versions, unit testing, CI with GitHub actions, and documentation with doxygen. These process elements will only fall into place when they are regarded as firm requirements.

The progress in the various NCEPLIBS projects demonstrates that this can be accomplished and very much improves quality and speeds future development.

SamuelTrahanNOAA referenced this issue in SamuelTrahanNOAA/fv3atm Jun 13, 2022
Changes in this PR:

* add a new regression test that uses the HWRF versions of saSAS deep and shallow convection
* update how the sutils module is loaded on hera and jet
* contains @MinsukJi-NOAA's unit testing branch
* contains @DusanJovic-NOAA's butterfly effect branch (changes in GFDL_atmos_cubed_sphere only)

Note: The changes in the ccpp-physics PR NCAR/ccpp-physics#423 lead to different results for two existing regression tests in PROD mode: fv3_ccpp_regional_c768 and fv3_ccpp_stretched_nest. In a separate comment below, I will describe in detail my investigation that allowed me to conclude that this change is acceptable.

Co-authored-by: MinsukJi-NOAA <Minsuk.Ji@noaa.gov>
Co-authored-by: Dusan Jovic <dusan.jovic@noaa.gov>
SamuelTrahanNOAA referenced this issue in SamuelTrahanNOAA/fv3atm Jun 13, 2022
…changes_combined_20210712

Wrapper PR for NCAR#94 (GF aerosol updates and tunings)
LarissaReames-NOAA pushed a commit to LarissaReames-NOAA/fv3atm that referenced this issue Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants