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 med_enthaply_mod #404

Draft
wants to merge 38 commits into
base: main
Choose a base branch
from
Draft

Conversation

jedwards4b
Copy link
Collaborator

@jedwards4b jedwards4b commented Aug 29, 2023

Description of changes

Update calculation of enthalpy and include for F cases.

Specific notes

Contributors other than yourself, if any: Mariana Vertenstein, Gustavo Marques, Peter Lauritzen

CMEPS Issues Fixed (include github issue #):

Are changes expected to change answers? yes

Any User Interface Changes (namelist or namelist defaults changes)?
Added compute_enthalpy to driver namelist.

Testing performed

Please describe the tests along with the target model and machine(s)
If possible, please also added hashes that were used in the testing

@jedwards4b jedwards4b marked this pull request as draft August 29, 2023 23:59
@jedwards4b
Copy link
Collaborator Author

Tested against b.e23_alpha16b.BLT1850.ne30_t232.040. results are bfb.

@jedwards4b jedwards4b self-assigned this Aug 30, 2023
@jedwards4b
Copy link
Collaborator Author

This is now functional for an f case. I added a print of the global enthalphy correction value to the mediator log file. Values over a 5 day run of ne30pg3_ne30pg3_mg17.FMTHIST_v0d are:
0.0 on the first step
-0.66 on the second step
then ranging between 0 and 1 over the rest of the run.

@jedwards4b
Copy link
Collaborator Author

Values for the B case are very similar.

mediator/med_enthalpy_mod.F90 Outdated Show resolved Hide resolved
mediator/med_enthalpy_mod.F90 Outdated Show resolved Hide resolved
Copy link
Collaborator

@mvertens mvertens left a comment

Choose a reason for hiding this comment

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

The nems changes need to be reverted - otherwise this is fine!

@tto061
Copy link

tto061 commented Sep 1, 2023

I think there are several problems with this code.

Gustavo and Peter and I have been discussing (a lot) on the scientifically correct and technically best way to implement the exchange material enthalpy fluxes. The main points we agreed on are:

  1. hrain and hsnow depend on atmospheric processes and can only be computed correctly in the atmospheric model component; for consistency also hevap should be computed there

  2. is it in fact not even possible to maintain energy consistency and (a fortiori) conservation in the atmosphere unless this is done

  3. so the mediator is the wrong place for these calculations; in particular the current calculation breaks atmospheric energy conservation even at a basic level because it uses a zero meterial enthalpy point of 0°C: the fluxes passed to each component always need to be re-referenced to the zero enthalpy point of that component

  4. even before all this, we had already decided to update the mediator to use the correct heat capacities for each water species

  5. finally, with atmospheric energy consistency in place, we only need to use the mediator fixer (g.a. correction to the sensible heat flux) for the run-off components (hrofi/l)

I have already written code, tested it (both on betzy and on cheyenne) and shared it with Gustavo and Peter that does what we regard to be the correct calculation, computing the hfluxes in the atmosphere model (although not in the final way we eventually wish it to be done) and passing them to the ocean. My code works in all F, G, and coupled cases, (only for G cases the hfluxes are computed in the way you do).

As Gustavo and Peter already know, you can find this as SourceMods in my case directory
/glade/work/toniazzo/4gustavo/cases/G/g.e23_b15.GJRAv4.TL319_t232_zstar_N65.toniazzo
(for now it does still lack the namelist and diagnostic output parts). I invited G&P to check and run tests of their own (I don't know if they did).

So it loosk to me like there's been some poor communication, and ill timing, here. I think this pull request should be revised. I would suggest to try a merge with what I've written, and I am entirely at your disposal for questions and to help out.

@jedwards4b
Copy link
Collaborator Author

@tto061 please review: jedwards4b#2

@gustavo-marques
Copy link
Collaborator

I am trying to run a G compset and I am getting SIGSEGV(11). Here is the cesm.log file:
/glade/scratch/gmarques/g.e23_b16.GJRAv4.TL319_t232_zstar_N65.enthalpy_case5/run/cesm.log.3391111.chadmin1.ib0.cheyenne.ucar.edu.230919-142440

@jedwards4b
Copy link
Collaborator Author

@gustavo-marques that issue should be fixed - update cmeps and try again.

@gustavo-marques
Copy link
Collaborator

Just confirming I was able to run a G compset

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