-
Notifications
You must be signed in to change notification settings - Fork 25
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
always obtain ndep from datm or cam using cmeps/cdeps #410
Conversation
hamocc/mo_read_ndep.F90
Outdated
!$omp parallel do private(i) | ||
do j=1,kpje | ||
do i=1,kpie | ||
ndep(i,j,idepnoy) = ndep(i,j,idepnoy) + ndep(i,j,idepnhx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit puzzled in this line: if use_extNcycle=.false.
, ndep
is of size (..,..,1)
and not (..,..,2)
- see mo_param1_bgc
. patmnoydep
and patmnhxdep
hold the fluxes coming from the nuopc cap, right? - hence I would suggest:
ndep(i,j,idepnoy) = (patmnoydep(i,j)+patmnhxdep(i,j))*fatmndep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I I am not mistaken, this would also affect the former loop above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jmaerz - thanks for catching this. You are totally right. I agree with your suggested change.
</values> | ||
<desc>if region includes arctic ocean</desc> | ||
</entry> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question out of curiosity: what is the use_arctic switch for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use_arctic has a different communication pattern in mod_xc.F90.
In particular - it checks that all arctic patch tiles must be the same size or empty,
and empty tiles must be "twinned" across the top boundary. It ensures that the latitudinal tile dimension is closed/arctic and handles the halo in the arctic differently. @matsbn - do you want to add more?
In the figures above (2nd test case): Something seem to be odd - the pattern look quite different and it doesn't seem having to do with the displayed ranges. |
|
||
! if N-deposition is switched off set ndep to zero and return | ||
if (.not. do_ndep) then | ||
ndep(:,:,:) = 0.0 | ||
return | ||
endif | ||
|
||
! If use_nuopc_ndep, nitrogen deposition is ALWAYS obtained from the | ||
! nuopc mediator | ||
if (use_extNcycle .and. do_ndep_coupled) then | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dear @mvertens , I am not fully getting your logics here. If I am not mistaken, the source of N-deposition (whether it comes from CAM or from input files) will be decided at the level of the nuopc, right - so the do_ndep_coupled
should appear somewhere else? - currently, the extended nitrogen cycle wouldn't be supported by nupc without the coupled fields to CAM, if I am not mistaken. If so, I would rewrite the code as follows (sorry for the formatting - it's a bit hard on github to keep the overview in a small text box):
if (use_nupc_ndep) then
! get N-deposition from atmosphere (either CAM or data)
ndep(:,:,:) = 0.
fatmndep = 365.*86400./mw_nitrogen
if (use_extNcycle) then
!$omp parallel do private(i)
do j=1,kpje
do i=1,kpie
! convert from kgN/m2/s to climatological input file units: kmolN/m2/yr
if (patmnoydep(i,j) > 0.) then
ndep(i,j,idepnoy) = patmnoydep(i,j)*fatmndep
endif
if (patmnhxdep(i,j) > 0.) then
ndep(i,j,idepnhx) = patmnhxdep(i,j)*fatmndep
endif
enddo
enddo
!$omp end parallel do
if (mnproc == 1 .and. first_call) then
write (io_stdo_bgc,*) 'iHAMOCC: getting NOy and NHx deposition from atm'
endif
else ! standard N-cycle version
!$omp parallel do private(i)
do j=1,kpje
do i=1,kpie
! convert from kgN/m2/s to climatological input file units: kmolN/m2/yr
! reduced and oxidized forms will all enter the NO3 pool
ndep(i,j,idepnoy) = (patmnoydep(i,j)+patmnhxdep(i,j))*fatmndep
enddo
enddo
!$omp end parallel do
end if
else ! MCT coupler part - don't using nuopc
! read ndep data from file
if (kplmon /= oldmonth) then
month_in_file=(max(startyear,min(endyear,kplyear))-startyear)*12+kplmon
if (mnproc == 1) then
write(io_stdo_bgc,*) 'Read N deposition month ',month_in_file,' from file ',trim(ndepfile)
endif
ncstat=nf90_open(trim(ndepfile),nf90_nowrite,ncid)
if (use_extNcycle) then
call read_netcdf_var(ncid,'nhxdep',nhxdepread,1,month_in_file,0)
call read_netcdf_var(ncid,'noydep',noydepread,1,month_in_file,0)
else
call read_netcdf_var(ncid,'ndep',ndepread,1,month_in_file,0)
endif
ncstat=nf90_close(ncid)
oldmonth=kplmon
endif
!$omp parallel do private(i)
do j=1,kpje
do i=1,kpie
if (use_extNcycle) then
ndep(i,j,idepnoy) = noydepread(i,j)
ndep(i,j,idepnhx) = nhxdepread(i,j)
else
ndep(i,j,idepnoy) = ndepread(i,j)
endif
enddo
enddo
!$omp end parallel do
endif
endif
@jmaerz - I think a chat at this point would be helpful. My logic was the following:
|
Hi @mvertens , as a follow up on our call: |
@Mats and @JorgSchwinger , just to inform you, I just discussed with @mvertens that for the optional extended nitrogen cycle, I don't see a need in |
@jmaerz - I think the code is behaving correctly at this point and the differences we are seeing are due to differences in the forcing datasets. I think this PR will ensure that CAM and CDEPS will determine what is sent for ndeps to BLOM and will be used consistently by both CTSM and BLOM. Are you okay with this PR - or do you still have concerns? I'm happy to run other tests. |
@TomasTorsvik @matsbn @JorgSchwinger - could you please provide feedback on this PR? I'd like to ensure that we have consistency for the NDEP received by BLOM and CLM and have CAM be control this forcing. |
hamocc/mo_param1_bgc.F90
Outdated
@@ -242,12 +242,12 @@ subroutine init_indices() | |||
use mo_control_bgc, only: bgc_namelist,get_bgc_namelist, io_stdo_bgc | |||
use mo_control_bgc, only: use_BROMO,use_AGG,use_WLIN,use_natDIC,use_CFC,use_cisonew, & | |||
use_sedbypass,use_PBGC_OCNP_TIMESTEP,use_PBGC_CK_TIMESTEP, & | |||
use_FB_BGC_OCE, use_BOXATM,use_extNcycle,use_pref_tracers | |||
use_FB_BGC_OCE, use_BOXATM,use_extNcycle,use_pref_tracers,use_nuopc_ndep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a line break here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mvertens , thanks for the work and thanks for the discussions on it. From my side, it looks ok now and can be merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mvertens - thanks! Look fine as far as I can see.
Forgot to add my review. Doing it now.
@mvertens - I see you haven't assigned anyone for merging this PR. I can do it if you want. |
We discussed to wait for @JorgSchwinger 's approval/last check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I agree that it becomes complicate to support several options. So I would support to remove the possibility to read ndep from file (inside HAMOCC) in NorESM2.5
do j=1,kpje | ||
do i=1,kpie | ||
! reduced and oxidized forms will all enter the NO3 pool | ||
ndep(i,j,idepnoy) = (patmnoydep(i,j)+patmnhxdep(i,j))*fatmndep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is here no if-block patmnoydep(i,j) > 0. .and. patmnhxdep(i,j) > 0.
? Either this is needed (then it should be here) or it isn't needed, but then it also could be removed in the block above for better readability and consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mvertens and @JorgSchwinger , the if
statements above were introduced at some point due to how it was written formerly in mo_hamocc4bcm
- not sure, if it was really needed there for the online coupling to the atmosphere (it was handled this way also for bromoforme and I simply oriented along it). If CAM has always values >= zero,. then it can be removed, otherwise, it would be good to introduce it here as well.
|
||
! Namelist input | ||
logical :: use_diag = .false. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the right place to define this variable? Where in BLOM are the other namelist variables defined?
Hi @mvertens , after the discussion with Jörg, I feel free to perform this PR (I hope that's ok with you). I will afterward introduce the small |
@jmaerz - thanks for merging this! I think your plan sounds good.
The simplest way to remove the possibility to read in ndep for noresm2.5 is to only support nuopc capability in master. I think it will get more and more confusing to have all the older namelists that are needed only by mct and the new namelists needed by nuopc. This will be even more evident as I bring in changes for using streams to read in other forcing datasets. I'm not sure when we will be okay with splitting master off - but it would be good to discuss this in an upcoming meeting. |
In noresm2_5, ndep is always sent to blom by either cam or datm - so there is no need to read a file.
This PR implements the functionality in blom to use this import data rather than reading a file.
use_nuopc_ndep
) has been added to the namelist groupconfig_bgc
Test 1: ERS_Ld3.ne30pg3_tn14.N1850.betzy_intel.allactive-defaultio
The left hand screen is the dev1.6.1.8 output for ndep whereas the right hand screen is the new output for ndep
The two have different behavior. The difference is due to the differerent ndep forcing files:
/cluster/shared/noresm/inputdata/ocn/blom/bndcon/ndep_1850_CMIP6_tnx1v4_20171106.nc
/cluster/shared/noresm/inputdata/lnd/clm2/ndepdata/fndep_clm_WACCM6_CMIP6piControl001_y21-50avg_1850monthly_0.95x1.25_c180802.nc
was obtained from an original fv1.9x2.5 dataset whereas cam is read in an fv09.x1.25 dataset.
Test 2: ERS_Ld3.TL319_tn14.NOIIAJRAOC.betzy_intel.blom-hybrid
/cluster/shared/noresm/inputdata/lnd/clm2/ndepdata/fndep_clm_hist_b.e21.BWHIST.f09_g17.CMIP6-historical-WACCM.ensmean_1849-2015_monthly_0.9x1.25_c180926.nc
/cluster/shared/noresm/inputdata/ocn/blom/bndcon/ndep_1850_CMIP6_tnx1v4_20171106.nc
But this is incorrect since the compset is for 2000 (see ndep forcing file is incorrect for most compsets using DATM #411)
xmlchange DATM_PRESNDEP=clim_1850
now results in the following two comparisons