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

Physics cleanup #37

Merged
merged 10 commits into from
Sep 30, 2023

Conversation

eclare108213
Copy link
Owner

Implement calls to icepack instead of colpkg for nonstandard configurations: ISPOL, uniform_1D, single_cell.

Set a single value for pi. There are still 3 different variables: pi, pii, seaicePi (pi and pii are parameters; seaicePi is not).

Check that emissivity=1 in coupled configurations and abort if it is not.

Deprecate cesm ponds, 0-layer thermo and zsalinity. These are fully deprecated in icepack, although the MPAS-SI icepack driver still includes some active zsalinity arguments for BGC; others are commented out in case we need them. Icepack aborts if any of them are turned on. The column driver also aborts if any of these are turned on, but the code is still available if the abort is manually removed. Not tested.

Update authorship in the icepack driver.

BFB in 3-month D cases.

@eclare108213
Copy link
Owner Author

@njeffery: The zsalinity changes might cause the code to fail in BGC configurations, especially in standalone cases. The build system is very confusing - not sure what's for standalone and what isn't. I'm happy to fix any problems in this PR or we can do it later.

@eclare108213
Copy link
Owner Author

@proteanplanet: I created a new subroutine where constant values can be checked against limits or other constraints. The only thing in there at the moment is emissivity.

Copy link
Collaborator

@proteanplanet proteanplanet left a comment

Choose a reason for hiding this comment

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

Visually inspected. BFB in D-case is a good test for these changes.

@njeffery
Copy link
Collaborator

@eclare108213 : stand alone running of mpas-seaice is broken because "use shr_const_mod" can only be done in coupled mode. That's why we had two directories for the column constants (cesm and cice). This was merged in a previous PR in mpas_seaice_constants.F90.

@eclare108213
Copy link
Owner Author

@eclare108213 : stand alone running of mpas-seaice is broken because "use shr_const_mod" can only be done in coupled mode. That's why we had two directories for the column constants (cesm and cice). This was merged in a previous PR in mpas_seaice_constants.F90.

Ah, okay, I think I can fix that using #ifdef CCSMCOUPLED. Is that the only thing broken?

@njeffery
Copy link
Collaborator

@eclare108213 : you'll need to wrap the constant definitions as well. Anything with SHR_CONS_... needs an alternative.

@eclare108213
Copy link
Owner Author

But those are already wrapped, aren't they? Am I missing something?

@njeffery
Copy link
Collaborator

You're right! my mistake. I'll try again

@njeffery
Copy link
Collaborator

Missing a ,& after the pi definition
L62 pi = 3.14159265358979323846_RKIND, &

@njeffery
Copy link
Collaborator

Need to define seaiceLatentHeatSublimation before seaiceLatentHeatMelting

@eclare108213
Copy link
Owner Author

Need to define seaiceLatentHeatSublimation before seaiceLatentHeatMelting

I think it is?

I fixed the pi continuation line, will push when you get through the compile stage. Just let me know what else to fix. thx

@njeffery
Copy link
Collaborator

@eclare108213 : the problem seems to be that seaiceLatentHeatMelting can't be defined in terms of seaiceLatentHeatSublimation and seaiceLatentHeatVaporization because they are not parameters.

This works fine
seaiceLatentHeatMelting = 3.34e5_RKIND

@njeffery
Copy link
Collaborator

@eclare108213 : one more thing at it'll compile. The Makefile in analysis_members needs
-I../icepack/columnphysics

@eclare108213
Copy link
Owner Author

I've made those changes. Fingers crossed.

@eclare108213
Copy link
Owner Author

This PR remains BFB after the recent commits:

 D3.nset05.emc.physics_cleanup2.physics_cleanup.eclare108213.anvil k000 - 
    D3.nset05.emc.physics_cleanup2.icepack-integration.eclare108213.anvil k000:

    iceAreaCell:  	BFB
    iceVolumeCell:  	BFB
    icePressure:  	BFB
    uVelocityGeo:  	BFB
    vVelocityGeo:  	BFB

    totalIceExtent:  	BFB
    totalIceVolume:  	BFB
    totalSnowVolume:  	BFB
    totalKineticEnergy:  	BFB
    averageAlbedo:  	BFB


    D3.nset05.emc.physics_cleanup2.physics_cleanup.eclare108213.anvil k001 - 
    D3.nset05.emc.physics_cleanup2.icepack-integration.eclare108213.anvil k001:

    iceAreaCell:  	BFB
    iceVolumeCell:  	BFB
    icePressure:  	BFB
    uVelocityGeo:  	BFB
    vVelocityGeo:  	BFB

    totalIceExtent:  	BFB
    totalIceVolume:  	BFB
    totalSnowVolume:  	BFB
    totalKineticEnergy:  	BFB
    averageAlbedo:  	BFB

@eclare108213
Copy link
Owner Author

@njeffery are you okay with this PR as it is, or do you want me to wait a little longer before merging it? I think we'll need to merge it by Monday morning at the latest, preferably sooner.

@njeffery
Copy link
Collaborator

@eclare108213 : I'm getting a runtime error when I run stand-alone with the 'icepack' option and standard physics+snow+snicar_ad

Fortran runtime error: Actual string length does not match the declared one for dummy argument 'warningsout' (5
12/256)

Error termination. Backtrace:
#0 0xcb4afd in __icepack_warnings_MOD_icepack_warnings_getall
▶ at /gpfs/fs1/home/ac.jeffery/E3SMv2/code/20230824/components/mpas-seaice/src/icepack/columnphysics/icep
ack_warnings.F90:96
#1 0xae2a8f in __seaice_icepack_MOD_seaice_icepack_write_warnings
▶ at /home/ac.jeffery/E3SMv2/code/20230824/components/mpas-seaice/src/shared/mpas_seaice_icepack.F:17350
#2 0xb5f444 in __seaice_icepack_MOD_seaice_init_icepack_constants
▶ at /home/ac.jeffery/E3SMv2/code/20230824/components/mpas-seaice/src/shared/mpas_seaice_icepack.F:5745
#3 0xababc0 in __seaice_initialize_MOD_seaice_init
▶ at /home/ac.jeffery/E3SMv2/code/20230824/components/mpas-seaice/src/shared/mpas_seaice_initialize.F:96
#4 0x920478 in mpas_init_block
▶ at /home/ac.jeffery/E3SMv2/code/20230824/components/mpas-seaice/src/model_forward/mpas_seaice_core.F:24
0

@eclare108213
Copy link
Owner Author

Hmmm, I've run into that kind of problem before. MPAS seems to have some sort of internal character definition that throws errors if you try to use some other string length. I don't know what it is or what it should be -- I ended up defining a temporary character string for (I think it was the shortwave config) sent into Icepack. Very frustrating. But I don't understand why you would be getting this error in standalone when we don't get it in coupled runs.

@njeffery
Copy link
Collaborator

@eclare108213 : I'll look into it. By the way, this is with eclare1082131/seaice/ice_integration not physics_cleanup though I imagine it's in both. I'm testing you're branch now.

@eclare108213
Copy link
Owner Author

Try this:
In subroutine seaice_icepack_write_warnings in mpas_seaice_icepack.F, change
character(len=strKINDWarnings), dimension(:), allocatable :: &
to
character(len=256), dimension(:), allocatable :: &
If that works, maybe we can define a special character string length in mpas for icepack warnings. Icepack defines char_len_long=256.

@eclare108213
Copy link
Owner Author

If you get errors in the subroutine above that (column_write_warnings), just delete it.

@njeffery
Copy link
Collaborator

@eclare108213 : That worked!

Copy link
Collaborator

@njeffery njeffery left a comment

Choose a reason for hiding this comment

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

Approved with the following change to icepack warnings:

  • character(len=strKINDWarnings), dimension(:), allocatable :: &
  • character(len=256), dimension(:), allocatable :: &
    warnings

Tested in stand-alone mode with icepack and column_package options. Changes to column are BFB and differences between icepack and column_package options remain small.

@eclare108213
Copy link
Owner Author

Verified that the latest change compiles, runs and remains BFB.

@eclare108213 eclare108213 merged commit 50b3913 into eclare108213/seaice/icepack-integration Sep 30, 2023
@eclare108213 eclare108213 deleted the physics_cleanup branch November 8, 2023 16:16
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