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 drying_slope convergence tests #703

Merged
merged 16 commits into from
Oct 25, 2023

Conversation

cbegeman
Copy link
Collaborator

@cbegeman cbegeman commented Sep 26, 2023

This PR adds a convergence test case for both wetting-and-drying methods, standard and ramp. The RMSE is computed at all locations and times for which an analytic solution is available (the analytic solution was provided prior to this PR). It produces a convergence plot and compares both methods when both have been run. The analytic solution is only comparable to the default cases, which use Rayleigh damping.

Checklist

  • User's Guide has been updated
  • Developer's Guide has been updated
  • API documentation in the Developer's Guide (api.rst) has any new or modified class, method and/or functions listed
  • Documentation has been built locally and changes look as expected
  • Document (in a comment titled Testing in this PR) any testing that was used to verify the changes
  • New tests have been added to a test suite

@cbegeman cbegeman marked this pull request as draft September 26, 2023 15:21
@cbegeman
Copy link
Collaborator Author

Testing

All new tests and the wetdry suite have been run on Chrysalis with intel, openmpi. The time step has been changed so the cases are no longer BFB with main. The only other namelist change is widening the ramp layer thickness range (only affecting ramp cases).

@cbegeman
Copy link
Collaborator Author

cbegeman commented Sep 26, 2023

The convergence plot generate by new tests:
image
There are no differences between standard and ramp methods.

I believe that this is due to the fact that neither the thin film region nor the exact location of the wetting front are included in the RMSE computation. The analytic solution is only defined for a few points in the wet regions.

@cbegeman cbegeman force-pushed the enhance-drying-slope-with-convergence branch from 06eca72 to 16eee6b Compare September 26, 2023 15:53
@cbegeman cbegeman marked this pull request as ready for review September 26, 2023 16:35
@cbegeman cbegeman requested review from xylar and sbrus89 September 26, 2023 16:36
@cbegeman cbegeman self-assigned this Sep 26, 2023
@cbegeman cbegeman added enhancement New feature or request ocean labels Sep 26, 2023
@xylar
Copy link
Collaborator

xylar commented Sep 28, 2023

@cbegeman, sorry, I lost track of this one. I'll try to give it a look today or tomorrow, but I might not get to it until early next week.

@cbegeman
Copy link
Collaborator Author

@xylar Next week is fine. Thanks!

@cbegeman cbegeman force-pushed the enhance-drying-slope-with-convergence branch 2 times, most recently from 27835c6 to 9fe3988 Compare October 3, 2023 22:13
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 great to me. I'm happy to do some testing but I'm also happy to trust what's been done already.

@cbegeman
Copy link
Collaborator Author

cbegeman commented Oct 4, 2023

@xylar Thank you for reviewing!

@cbegeman
Copy link
Collaborator Author

@sbrus89 Do you have time to review this PR? I'm about to open another that is based on these changes. No worries if not.

@cbegeman cbegeman force-pushed the enhance-drying-slope-with-convergence branch from 9fe3988 to 8d7bd16 Compare October 11, 2023 20:32
@sbrus89
Copy link
Collaborator

sbrus89 commented Oct 11, 2023

@cbegeman, sorry for being slow on this. I'm running tests now.

@cbegeman
Copy link
Collaborator Author

@sbrus89 Thanks!

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.

@cbegeman, this looks great. I just a few comments...

@cbegeman
Copy link
Collaborator Author

@sbrus89 Thanks! I'll get to this when I get back from vacation early next week.

@cbegeman cbegeman force-pushed the enhance-drying-slope-with-convergence branch from 8d7bd16 to 58a577b Compare October 16, 2023 20:04
@cbegeman cbegeman force-pushed the enhance-drying-slope-with-convergence branch from 58a577b to bcb9f2e Compare October 16, 2023 20:10
@cbegeman cbegeman force-pushed the enhance-drying-slope-with-convergence branch from 76f4b0f to 8e2af4a Compare October 17, 2023 19:16
@cbegeman
Copy link
Collaborator Author

@sbrus89 This is ready for your re-review. I have made the suggested changes, including significant clean-up of the viz step. Let me know if you feel strongly about making hmax a config option.

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.

@cbegeman, this looks great to me. Approving based on testing the wetdry suite on Perlmutter.

@sbrus89
Copy link
Collaborator

sbrus89 commented Oct 25, 2023

On a note unrelated to this PR, I did see some out-of-bounds issues for the loglaw cases using debug flags.

ocean/drying_slope/sigma/standard/250m/loglaw
  * step: initial_state
  * step: forward
      Failed
  test execution:      ERROR
  see: case_outputs/ocean_drying_slope_sigma_standard_250m_loglaw.log
  test runtime:        00:36
ocean/drying_slope/sigma/standard/1km/loglaw
  * step: initial_state
  * step: forward
      Failed
  test execution:      ERROR
  see: case_outputs/ocean_drying_slope_sigma_standard_1km_loglaw.log
  test runtime:        00:06
ocean/drying_slope/single_layer/standard/250m/loglaw
  * step: initial_state
  * step: forward
      Failed
  test execution:      ERROR
  see: case_outputs/ocean_drying_slope_single_layer_standard_250m_loglaw.log
  test runtime:        00:03
ocean/drying_slope/single_layer/standard/1km/loglaw
  * step: initial_state
  * step: forward
      Failed
  test execution:      ERROR
  see: case_outputs/ocean_drying_slope_single_layer_standard_1km_loglaw.log
  test runtime:        00:04

It seems like the issue is here: https://github.com/E3SM-Project/E3SM/blame/15fa673c5b8d985f177e746c8d3b68c3b0b1e756/components/mpas-ocean/src/shared/mpas_ocn_vmix.F#L1072-L1074. Here is the backtrace on the forward run:

At line 1074 of file mpas_ocn_vmix.F
Fortran runtime error: Index '0' of dimension 1 of array 'kineticenergycell' below lower bound of 1

Error termination. Backtrace:
At line 1074 of file mpas_ocn_vmix.F
Fortran runtime error: Index '0' of dimension 1 of array 'kineticenergycell' below lower bound of 1

Error termination. Backtrace:
At line 1074 of file mpas_ocn_vmix.F
Fortran runtime error: Index '0' of dimension 1 of array 'kineticenergycell' below lower bound of 1

Error termination. Backtrace:
At line 1074 of file mpas_ocn_vmix.F
Fortran runtime error: Index '0' of dimension 1 of array 'kineticenergycell' below lower bound of 1

Error termination. Backtrace:
#0  0xc697ea in ocn_vel_vmix_tend_implicit_spatially_variable_loglaw
        at /global/cfs/cdirs/e3sm/sbrus/compass_worktrees/drying_slope_convergence/E3SM-Project/components/mpas-ocean/src/shared/mpas_ocn_vmix.F:1074
#1  0xc66d2b in __ocn_vmix_MOD_ocn_vmix_implicit
        at /global/cfs/cdirs/e3sm/sbrus/compass_worktrees/drying_slope_convergence/E3SM-Project/components/mpas-ocean/src/shared/mpas_ocn_vmix.F:1515
#0  0xc697ea in ocn_vel_vmix_tend_implicit_spatially_variable_loglaw

@cbegeman
Copy link
Collaborator Author

@sbrus89, Thanks for your review! @gcapodag pointed out that issue here: #722. I have a fix that I'm testing.

@cbegeman cbegeman merged commit 50395b4 into MPAS-Dev:main Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ocean
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants