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 interface variables to nc output in standalone homme #3078

Merged
merged 36 commits into from
Oct 14, 2019

Conversation

oksanaguba
Copy link
Contributor

@oksanaguba oksanaguba commented Jul 18, 2019

Adds native and interpolated output of interface quantities in homme, in particular, for w and mu (dpnh/dphy). Fixes std output for interface quantities to have min/max calculated on interfaces, not averaged level values.

[bfb], affects only standalone homme.

@oksanaguba
Copy link
Contributor Author

sanity check -- from plotting both wnlev and wnlevp they look the same (up to the bottom point).

@rljacob rljacob added the HOMME label Jul 19, 2019
@mt5555 mt5555 added HOMME standalone issues with the standalone HOMME code that dont impact E3SM and removed HOMME labels Jul 24, 2019
@oksanaguba
Copy link
Contributor Author

oksanaguba commented Sep 12, 2019

I ran ne30 HS test for 10 days and looked at both interp (ncview) and native (ncl) output for geo_i, w_i, mu_i . I see nothing alarming. @mt5555 do you have more suggestions about this code? I will run homme suite with it.

@oksanaguba oksanaguba changed the title [wip] w on interfaces in homme movies Adds interface quantities to nc output in standalone homme Sep 12, 2019
@oksanaguba oksanaguba changed the title Adds interface quantities to nc output in standalone homme Add interface quantities to nc output in standalone homme Sep 13, 2019
@oksanaguba
Copy link
Contributor Author

By some reason baroCamMoistSL ran on anvil without issues in this branch and commit it branched off.

@oksanaguba oksanaguba changed the title Add interface quantities to nc output in standalone homme Add interface variables to nc output in standalone homme Sep 21, 2019
@oksanaguba
Copy link
Contributor Author

SW tests were fixed, this is ready.

@oksanaguba oksanaguba added the BFB PR leaves answers BFB label Sep 21, 2019
@oksanaguba
Copy link
Contributor Author

Reran previous interp and native output with the newest commit and files are identical (folder /home/onguba/as/run-test-wmovie-pr/movies ).

call nf_variable_attributes(ncdf, 'mu_i', 'mu=dp/d\pi on interfaces','dimensionless')
call nf_variable_attributes(ncdf, 'geo_i','geopotential on interfaces','meters')
call nf_variable_attributes(ncdf, 'pnh', 'total pressure','Pa')
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

can you remove all the #ifdef MODEL_THETA_L's from this file? One of the reasons behind the "get_field()" interface was so we wouldn't need ifdef's. The preqx model will abort if someone requests a variable that is not supported, like w_i.

so lets also add the get_field_i() routine to preqx.

@@ -188,6 +191,7 @@ subroutine prim_movie_init(elem, par, hvcoord,tl)
compDOF(1:1),IOdescT)

deallocate(compdof)
deallocate(compdofp1)
Copy link
Contributor

Choose a reason for hiding this comment

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

note how the 2d and 3D arrays are both using compdof. can we do the same here and bump up the dimension of compdof and you dont need to introduce compdofp1?

call nf_variable_attributes(ncdf, 'mu_i', 'mu=dp/d\pi on interfaces','dimensionless')
call nf_variable_attributes(ncdf, 'geo_i','geopotential on interfaces','meters')
call nf_variable_attributes(ncdf, 'pnh', 'total pressure','Pa')
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

per previous comment - can we avoid all #ifdef MODEL_THETA_L in this file

elem(ie)%derived%mubottom(:,:)=1.0
enddo
#endif

Copy link
Contributor

Choose a reason for hiding this comment

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

this is getting complicated - I think if you really need mu at the surface we need to introduce a subroutine to compute it and it should be called by the I/O and by prim_advance_mod.F90

field = elem%state%phinh_i(:,:,1:nlevp,nt)
case('mu');
call get_dpnh_dp_i(elem,field,hvcoord,nt)
case default
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't the names be "w_i", since someone may ask for "w" on midpoints and "w_i" on interfaces?
there are existing NCL scripts that will break if 'w' and 'geo' output is now at interfaces instead of midpoints.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do. I removed w from interp_output but i will put it back. geo and geo_i are already there.

call pnh_and_exner_from_eos(hvcoord,elem%state%vtheta_dp(:,:,:,nt),&
dp,elem%state%phinh_i(:,:,:,nt),pnh,exner,dpnh_dp_i)

dpnh_dp_i(:,:,nlevp) = elem%derived%mubottom(:,:)
Copy link
Contributor

Choose a reason for hiding this comment

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

see comment above - i think this is too difficult to maintain

@@ -80,6 +79,8 @@ module element_state
real (kind=real_kind) :: FQps(np,np) ! forcing of FQ on ps_v

real (kind=real_kind) :: gradphis(np,np,2) ! grad phi at the surface, computed once in model initialization

real (kind=real_kind) :: mubottom(np,np) !mu
Copy link
Contributor

Choose a reason for hiding this comment

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

another problem with this approach: mu will be at the wrong time level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why? mubottom is obtained in caar and not changed in dirk, or by remap. or does it get changed by remap?

@oksanaguba
Copy link
Contributor Author

Homme suite is ok, it is ready to be reviewed again @mt5555 .

@oksanaguba oksanaguba assigned oksanaguba and unassigned mt5555 Oct 1, 2019
@oksanaguba
Copy link
Contributor Author

Assigning to myself since it was approved, unless there are objections.

@rljacob
Copy link
Member

rljacob commented Oct 10, 2019

@oksanaguba please merge this.

oksanaguba added a commit that referenced this pull request Oct 11, 2019
Adds native and interpolated output of interface quantities in homme,
in particular, for w and mu (dpnh/dphy).
Fixes std output for interface quantities to have min/max calculated
on interfaces, not averaged level values.

[bfb], affects only standalone homme.
@oksanaguba
Copy link
Contributor Author

Merged to next.

oksanaguba added a commit that referenced this pull request Oct 14, 2019
Adds native and interpolated output of interface quantities in homme,
in particular, for w and mu (dpnh/dphy).
Fixes std output for interface quantities to have min/max calculated
on interfaces, not averaged level values.

[bfb], affects only standalone homme.
@oksanaguba oksanaguba merged commit 157b397 into master Oct 14, 2019
@oksanaguba oksanaguba deleted the oksanaguba/homme/w-movie branch October 14, 2019 15:58
rljacob pushed a commit that referenced this pull request May 6, 2021
When using pio2 if the pio_numiotasks and pio_stride indicate processors above the model task count you will get an error. This change prevents that happening.

Test suite: by hand SMS_P45.f19_g16.X
Test baseline:
Test namelist changes:
Test status: bit for bit
Fixes

User interface changes?:

Update gh-pages html (Y/N)?:

Code review:
jgfouca pushed a commit that referenced this pull request Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB HOMME standalone issues with the standalone HOMME code that dont impact E3SM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants