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

Cleanup ocean bottom drag namelist options #551

Merged
merged 9 commits into from
Apr 29, 2023

Conversation

cbegeman
Copy link
Collaborator

@cbegeman cbegeman commented Mar 6, 2023

This PR updates compass in accordance with E3SM-Ocean-Discussion/E3SM#41. It results in different behavior for the QU240 test cases: explicit drag is no longer used and constant-coefficient implicit drag is retained (the default).

Checklist

  • The E3SM-Project submodule has been updated with relevant E3SM changes
  • Document (in a comment titled Testing in this PR) any testing that was used to verify the changes

@cbegeman
Copy link
Collaborator Author

cbegeman commented Mar 6, 2023

Testing

All of the test cases for which the namelist changes occurred were run with intel, openmpi on chrysalis (with the exception of ARRM). The QU240 test cases (I ran performance and dynamic adjustment tests) are non-BFB and so is the drying_slope/*km/sigma/default case (due to a bugfix).

@xylar xylar self-assigned this Mar 29, 2023
@cbegeman cbegeman force-pushed the ocn-cleanup-drag-config branch 2 times, most recently from 2b12c5f to 5fdbca8 Compare April 6, 2023 17:20
@xylar xylar mentioned this pull request Apr 13, 2023
5 tasks
@cbegeman
Copy link
Collaborator Author

cbegeman commented Apr 26, 2023

@scalandr and @jonbob Those tests are failing because you are using a more recent version of E3SM than compass's E3SM submodule. Once this PR is merged, you will be able to run compass with a more recent version of E3SM.

@scalandr I will rebase this branch onto main and then you can check it out for your tests.

@cbegeman cbegeman force-pushed the ocn-cleanup-drag-config branch from 5fdbca8 to 2b76a2c Compare April 26, 2023 18:17
@xylar
Copy link
Collaborator

xylar commented Apr 26, 2023

@cbegeman, I'm working on testing this now, and also doing some tests on the E3SM PRs between the last E3SM-Project submodule update (#606) and the update here, just to make sure there are no non-BFB surprises. I'll post back when I can.

@cbegeman
Copy link
Collaborator Author

@xylar Thanks for doing that testing! Much appreciated :)

@xylar
Copy link
Collaborator

xylar commented Apr 26, 2023

and also doing some tests on the E3SM PRs between the last E3SM-Project submodule update (#606) and the update here, just to make sure there are no non-BFB surprises.

Okay, so the answer here is that there aren't any ocean or framework PRs since #606, so the submodule update can proceed without any intermediate testing!

@xylar
Copy link
Collaborator

xylar commented Apr 26, 2023

A setback unrelated to this PR. I forgot to make new cache files after #610. Doing that now so I can run the pr suite.

@xylar
Copy link
Collaborator

xylar commented Apr 26, 2023

I'm stuck in the queue on Chrysalis, so will pick up where I left off tomorrow. In the meantime, it looks like this needs another rebase to fix a conflict with #619.

@cbegeman cbegeman force-pushed the ocn-cleanup-drag-config branch from 2b76a2c to 91fb959 Compare April 26, 2023 21:52
@cbegeman
Copy link
Collaborator Author

@xylar Done. Thanks!

@xylar
Copy link
Collaborator

xylar commented Apr 26, 2023

I'm seeing:

  ocean/isomip_plus/planar/2km/z-star/Ocean0
Warning: config_land_ice_flux_topDragCoeff is not in the namelist and replacements will not be used
Warning: config_land_ice_flux_topDragCoeff is not in the namelist and replacements will not be used
Warning: config_land_ice_flux_topDragCoeff is not in the namelist and replacements will not be used

Is that related to this work and could it be cleaned up here? Or do we need a different PR to fix that?

@xylar
Copy link
Collaborator

xylar commented Apr 26, 2023

To answer my own question, that's top drag and we're dealing with bottom drag here so no, this isn't the right PR. Could you make a quick one to fix this when you have time, @cbegeman, if we don't already have one I've lost track of?

Copy link
Collaborator

@xylar xylar left a comment

Choose a reason for hiding this comment

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

This looks good. In the pr suite, I'm seeing:

Test Runtimes:
00:04 PASS ocean_baroclinic_channel_10km_default
00:03 PASS ocean_baroclinic_channel_10km_threads_test
00:03 PASS ocean_baroclinic_channel_10km_decomp_test
00:03 PASS ocean_baroclinic_channel_10km_restart_test
00:02 PASS ocean_internal_wave_default
00:01 PASS ocean_internal_wave_vlr_default
02:43 PASS ocean_global_convergence_qu_cosine_bell
01:00 PASS ocean_global_ocean_QU240_mesh
00:48 PASS ocean_global_ocean_QU240_WOA23_init
00:31 FAIL ocean_global_ocean_QU240_WOA23_performance_test
01:02 FAIL ocean_global_ocean_QU240_WOA23_restart_test
01:02 FAIL ocean_global_ocean_QU240_WOA23_decomp_test
01:02 FAIL ocean_global_ocean_QU240_WOA23_threads_test
00:39 FAIL ocean_global_ocean_QU240_WOA23_analysis_test
01:06 FAIL ocean_global_ocean_QU240_WOA23_dynamic_adjustment
00:31 FAIL ocean_global_ocean_QU240_WOA23_RK4_performance_test
01:03 FAIL ocean_global_ocean_QU240_WOA23_RK4_restart_test
01:01 FAIL ocean_global_ocean_QU240_WOA23_RK4_decomp_test
01:02 FAIL ocean_global_ocean_QU240_WOA23_RK4_threads_test
00:00 PASS ocean_global_ocean_QUwISC240_mesh
00:01 PASS ocean_global_ocean_QUwISC240_WOA23_init
00:32 FAIL ocean_global_ocean_QUwISC240_WOA23_performance_test
00:00 PASS ocean_global_ocean_EC30to60_mesh
00:02 PASS ocean_global_ocean_EC30to60_WOA23_init
01:10 PASS ocean_global_ocean_EC30to60_WOA23_performance_test
00:00 PASS ocean_global_ocean_ECwISC30to60_mesh
00:03 PASS ocean_global_ocean_ECwISC30to60_WOA23_init
01:18 PASS ocean_global_ocean_ECwISC30to60_WOA23_performance_test
00:07 PASS ocean_ice_shelf_2d_5km_z-star_restart_test
00:08 PASS ocean_ice_shelf_2d_5km_z-level_restart_test
00:55 PASS ocean_isomip_plus_planar_2km_z-star_Ocean0
00:05 PASS ocean_ziso_20km_default
00:05 PASS ocean_ziso_20km_with_frazil
Total runtime 18:14
FAIL: 11 tests failed, see above.

The failures are in baseline validation and it sounds like they are expected. I looked at one of them and the differences were small (~1e-7, so not machine precision).

@xylar
Copy link
Collaborator

xylar commented Apr 26, 2023

This will need #621 before it can run the pr suite. My testing above included cherry-picking that commit.

@xylar xylar assigned cbegeman and unassigned xylar Apr 26, 2023
@xylar
Copy link
Collaborator

xylar commented Apr 26, 2023

@cbegeman, assigning this to you so you can merge once @sbrus89 has approved.

Copy link
Collaborator

@sbrus89 sbrus89 left a comment

Choose a reason for hiding this comment

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

Approved based on testing in conjunction with #620 and E3SM-Project/E3SM#5590 for the dam break and dying slope cases.

@xylar xylar merged commit 385b3e2 into MPAS-Dev:main Apr 29, 2023
@xylar
Copy link
Collaborator

xylar commented Apr 29, 2023

Thanks @cbegeman and @sbrus89!

@xylar xylar assigned xylar and unassigned cbegeman Apr 29, 2023
@xylar xylar mentioned this pull request May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants