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

Compile FMS with -DGFS_PHYS #29

Merged
merged 4 commits into from
Apr 22, 2020
Merged

Compile FMS with -DGFS_PHYS #29

merged 4 commits into from
Apr 22, 2020

Conversation

oliverwm1
Copy link
Contributor

Since we are compiling everything in FV3/ with the -DGFS_PHYS flag, we should do the same for FMS. This will slightly change the physical constants the model uses (and make the constants consistent with assumptions made by coarsening procedures in fv3net).

@oliverwm1
Copy link
Contributor Author

Partially addresses #28.

@rheacangeo
Copy link
Contributor

Thank you! I had noticed this when porting the FMS constants and thought it was strange. Making them consistent sounds good. I am surprised the checks passed though -- shouldn't you need to update the md5sums of the regression data?

Copy link
Contributor

@rheacangeo rheacangeo left a comment

Choose a reason for hiding this comment

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

I think the md5sums should probably be updated as well? But otherwise, I approve!

@oliverwm1
Copy link
Contributor Author

I think the md5sums should probably be updated as well? But otherwise, I approve!

Yeah I was also surprised (and concerned) that this passed the regression test. Looking at the CircleCI logs, it looks like a pre-built FMS image was pulled, which would explain why the test passed. I'll try running the test with docker layer caching off.

@oliverwm1
Copy link
Contributor Author

oliverwm1 commented Apr 22, 2020

I'll try running the test with docker layer caching off.

That didn't help, since the CircleCI config.yml still just calls make pull_deps and pulls the latest pre-built FMS, ESMF and SERIALBOX images and goes from there. If I understand correctly, the only way to have CircleCI test the model with a newer FMS version is to manually push the newer FMS image, but this seems a little risky, since then any other builds of fv3gfs-fortran or fv3gfs-python will use this latest version of FMS.

@oliverwm1
Copy link
Contributor Author

I confirmed that the regression tests were failing locally when using the new FMS image built with -DGFS_PHYS. I updated the md5sum files and now expect the tests to fail on CircleCI because CircleCI is using the old FMS image.

Copy link
Member

@spencerkclark spencerkclark left a comment

Choose a reason for hiding this comment

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

Looks good to me too. Nice work tracking this down!

@nbren12
Copy link
Contributor

nbren12 commented Apr 22, 2020

@oliverwm1
Copy link
Contributor Author

I manually pushed an updated FMS image and now rerunning CircleCI tests.

@oliverwm1 oliverwm1 merged commit 42f2877 into master Apr 22, 2020
@oliverwm1 oliverwm1 deleted the fix/FMS-build-flag branch April 22, 2020 19:22
nbren12 pushed a commit that referenced this pull request Apr 11, 2022
This PR updates the fortran sources from the older fv3gfs repo to the newer fv3atm repo. Running the model is the same, but small changes to the repo require different namelist options when running the model, so configuration dictionaries will need to be updated accordingly.

Other minor changes:

- Fixed name of "patch" version in RELEASE.rst
- Docker build instructions are used directly from the Fortran repo, instead of copying build instructions in two places.
- The new repo requires ESMF and FMS to be built separately
- 32bit builds are disabled, will require fixing the build steps in fv3gfs-fortran to properly pass 32bit build flags when building FMS. Flags can still be sent to fv3atm the same way as before.
- Fortran build sources are back to being a PHONY build target, because make was not able to follow through and build them.
- Regression tests are added that use the regression target data in the fv3gfs-fortran repo. This makes some of our other tests redundant, they may be removed later.
- Configuration dictionaries are explicitly specified for tests instead of using `get_default_config`.
- Commented-out restart tests are re-introduced as fixing restarts on the new repo is a work in progress.

Commit history:

* remove reference to old repo

* add reference to fv3atm repo

* first running docker build with fv3atm

* use ARG for root of fv3gfs build root in fortran image

* update permissions of fv3config cache directory to allow writes

* ignore DS_Store files

* add back legacy restart tests, disabled because not passing

* Add regression tests which use fv3gfs-fortran reference files

* update fv3config and fv3gfs-fortran to latest master

* update fv3config to v0.2.0

* only run tagged build on master

* bind-mount docker credentials for all tests

* fixed label for smallest version increment from bugfix to patch

* move image tests outside of in-docker test script

* remove redundant tagged-build workflow

* install python test dependencies on machine

* update pip and setuptools

* install fv3config for tests

* use local fv3config instead of git commit

* use python 2 for google cloud tools

* fix online_code example

* Added history entries
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 this pull request may close these issues.

4 participants