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

Add test that prescribing SST alters model evolution #256

Merged
merged 17 commits into from
Mar 25, 2021

Conversation

spencerkclark
Copy link
Member

@spencerkclark spencerkclark commented Mar 22, 2021

This PR adds a test confirming that with the use of the namelist flag added in ai2cm/fv3gfs-fortran#173, setting the "ocean_surface_temperature" modifies the evolution of the model's prognostic variables (e.g. air temperature).

In between ai2cm/fv3gfs-fortran#173 and ai2cm/fv3gfs-fortran#93, this functionality was broken (see the failing test associated with 5a6febb of this PR). In other words, since ai2cm/fv3gfs-fortran#93, Sfcprop%tsfco in the fortran model was modified within the physics driver prior to being used by any physics scheme; therefore when we set it via Python, it was immediately set to something else in the fortran before it could have any impact. The namelist parameter toggles the behavior back to what it was before ai2cm/fv3gfs-fortran#93, allowing us to prescribe the SST.

@spencerkclark spencerkclark changed the title [Do not merge] Add test demonstrating that old SST prescribing approach does not work Add test that prescribing SST alters model evolution Mar 22, 2021
@spencerkclark spencerkclark marked this pull request as ready for review March 23, 2021 16:16
Copy link
Contributor

@brianhenn brianhenn left a comment

Choose a reason for hiding this comment

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

Thanks @spencerkclark I only have a quick comment, I'll let @mcgibbon weigh in on the wrapper-specific stuff

)

# We expect these states to differ.
assert not np.array_equal(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a risk of this passing due to floating point error, i.e., would using np.allclose be better?

Copy link
Collaborator

@mcgibbon mcgibbon left a comment

Choose a reason for hiding this comment

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

There are some readability requests/suggestions, but I shouldn't need to re-review. Good job writing extensive tests!

tests/test_set_ocean_surface_temperature.py Outdated Show resolved Hide resolved
tests/test_set_ocean_surface_temperature.py Outdated Show resolved Hide resolved
tests/test_set_ocean_surface_temperature.py Outdated Show resolved Hide resolved
tests/test_set_ocean_surface_temperature.py Show resolved Hide resolved
tests/test_set_ocean_surface_temperature.py Outdated Show resolved Hide resolved
tests/test_set_ocean_surface_temperature.py Outdated Show resolved Hide resolved
@spencerkclark spencerkclark merged commit a66dae0 into master Mar 25, 2021
@spencerkclark spencerkclark deleted the test-sst-modification branch March 25, 2021 20:20
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.

3 participants