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.

Sign up for GitHub

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

Jump to bottom

Fix QC test, fix bug in history time axis, fix history output averaging for timestep output #624

Merged
merged 10 commits into from
Aug 19, 2021
Merged

Fix QC test, fix bug in history time axis, fix history output averaging for timestep output #624

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
441f693
Fix history features
apcraig Aug 10, 2021
15763d8
revert set_nml.histall
apcraig Aug 10, 2021
b3364a6
fix implementation error
apcraig Aug 11, 2021
96d5851
update model log output in ice_init
apcraig Aug 11, 2021
7b5c2b4
Fix QC issues
apcraig Aug 12, 2021
c5794b4
Update testing
apcraig Aug 13, 2021
eaa3c3a
update documentation
apcraig Aug 13, 2021
e31ce7e
reset calchk to 100000 years
apcraig Aug 13, 2021
83068c7
update evp1d test
apcraig Aug 13, 2021
3fd897e
Merge branch 'master' of https://github.com/cice-consortium/cice into…
apcraig Aug 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions cicecore/cicedynB/analysis/ice_history.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1758,6 +1758,7 @@ subroutine accum_hist (dt)
nstrm ! nstreams (1 if writing initial condition)

real (kind=dbl_kind) :: &
timedbl , & ! temporary dbl for time bounds
ravgct , & ! 1/avgct
ravgctz ! 1/avgct

Expand Down Expand Up @@ -1814,7 +1815,7 @@ subroutine accum_hist (dt)
n4Dfcum = n4Dscum + num_avail_hist_fields_4Df ! should equal num_avail_hist_fields_tot

do ns = 1,nstreams
if (.not. hist_avg .or. histfreq(ns) == '1') then ! write snapshots
if (.not. hist_avg) then ! write snapshots
Copy link
Contributor

Choose a reason for hiding this comment

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

At some point, I would like hist_avg to be an array of size nstreams. Then we can control the instantaneous versus average on a stream by stream basis.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea, I will make sure that's part of an issue.

do n = 1,n2D
if (avail_hist_fields(n)%vhistfreq == histfreq(ns)) &
a2D(:,:,n,:) = c0
Expand Down Expand Up @@ -1862,11 +1863,10 @@ subroutine accum_hist (dt)
avgct(ns) = c1
else ! write averages over time histfreq
avgct(ns) = avgct(ns) + c1
! if (avgct(ns) == c1) time_beg(ns) = (time-dt)/int(secday)
if (avgct(ns) == c1) then
time_beg(ns) = (timesecs-dt)/int(secday)
time_beg(ns) = real(time_beg(ns),kind=real_kind)
endif
endif
if (avgct(ns) == c1) then
Copy link
Contributor

Choose a reason for hiding this comment

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

So this just initializes timedbl and time_beg at the beginning of the averaging interval?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct. and timedbl is just a temporary local to make going from double to single a little cleaner. time_beg is now initialized whenever hist_avg is on because we should assume it might be written to the history file when hist_avg is on, even for histfreq='1' and histfreq_n=1. This moves away from treating histfreq='1' and histfreq_n=1 as special cases which allows for averaging across timesteps (histfreq='1') and allows a user to cleanly set hist_avg to true even if histfreq_n=1.

timedbl = (timesecs-dt)/(secday)
time_beg(ns) = real(timedbl,kind=real_kind)
Copy link
Member

Choose a reason for hiding this comment

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

we could do without timedbl and just do this, no ?

time_beg(ns) = real((timesecs-dt)/(secday), kind=real_kind)

Is there any reason that I'm missing why we need a separate variable ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We absolutely could implement what you propose.

endif
enddo

Expand Down Expand Up @@ -3966,8 +3966,8 @@ subroutine accum_hist (dt)
enddo ! iblk
!$OMP END PARALLEL DO

time_end(ns) = timesecs/int(secday)
time_end(ns) = real(time_end(ns),kind=real_kind)
timedbl = timesecs/secday
time_end(ns) = real(timedbl,kind=real_kind)

!---------------------------------------------------------------
! write file
Expand Down
97 changes: 50 additions & 47 deletions cicecore/cicedynB/analysis/ice_history_shared.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,64 +672,67 @@ subroutine construct_filename(ncfile,suffix,ns)
iday = mday
isec = msec - dt

if (write_ic) isec = msec
! construct filename
if (write_ic) then
isec = msec
write(ncfile,'(a,a,i4.4,a,i2.2,a,i2.2,a,i5.5,a,a)') &
incond_file(1:lenstr(incond_file)),'.',iyear,'-', &
imonth,'-',iday,'-',isec,'.',suffix
imonth,'-',iday,'-',isec,'.',trim(suffix)
else

if (hist_avg .and. histfreq(ns) /= '1') then
if (histfreq(ns) == 'h'.or.histfreq(ns) == 'H') then
! do nothing
elseif (new_year) then
iyear = iyear - 1
imonth = 12
iday = daymo(imonth)
elseif (new_month) then
imonth = mmonth - 1
iday = daymo(imonth)
elseif (new_day) then
iday = iday - 1
endif
endif

cstream = ''
if (hist_avg) then
if (histfreq(ns) == '1' .or. histfreq(ns) == 'h'.or.histfreq(ns) == 'H') then
! do nothing
elseif (new_year) then
iyear = iyear - 1
imonth = 12
iday = daymo(imonth)
elseif (new_month) then
imonth = mmonth - 1
iday = daymo(imonth)
elseif (new_day) then
iday = iday - 1
endif
endif
Comment on lines +683 to +696
Copy link
Member

Choose a reason for hiding this comment

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

When I looked at this code in the context of #551, I did not readily understand why we have to do those substractions here, and why we do not have to do any if histfreq = 1 or h...
Is it because the file names for averaged data are dated at the start of the averaging period ? I think a code comment explaining that would not hurt :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is all part of a rather complex interaction between dates and history frequencies with a certain amount of assumption built-in. Basically, if you are doing annual, monthly, or daily averages, the history will be written on the "new_flag". At that point, you're in the next year, month, or day. So you subtract "1" so when you construct the filename, the name is associated with the year, month, or day being averaged. Of course, all this comes unglued if the histfreq_n is not 1 in terms of the filename. The instantaneous stuff and histfreq='1' just writes out a file with the current date in it because there isn't a general way to create a meaningful filename. Most of what I did tried to preserve what's there and expand how it works. There are many things that could be further improved.


cstream = ''
!echmod ! this was implemented for CESM but it breaks post-processing software
!echmod ! of other groups (including RASM which uses CESMCOUPLED)
!echmod if (ns > 1) write(cstream,'(i1.1)') ns-1
Comment on lines +698 to 701
Copy link
Member

Choose a reason for hiding this comment

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

these lines (setting an empty cstream, the following comment and the commented line if (ns > 1) write(cstream,'(i1.1)') ns-1 ) dates all the way back to our initial commit 7d740de (initial commit of CICE without icepack to git from LANL consortium branch r1229 May 24, 2017, 2017-05-24). Maybe it would be time to just remove cstream ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about removing the cstream but they aren't hurting anyone and I wasn't sure whether maybe some coupled system was using them, so I left them in. We could/should think about removing it, but we'd want to ask outside users whether they are using it.


if (histfreq(ns) == '1') then ! instantaneous, write every dt
write(ncfile,'(a,a,i4.4,a,i2.2,a,i2.2,a,i5.5,a,a)') &
history_file(1:lenstr(history_file))//trim(cstream),'_inst.', &
iyear,'-',imonth,'-',iday,'-',msec,'.',suffix

elseif (hist_avg) then ! write averaged data

if (histfreq(ns) == 'd'.or.histfreq(ns) == 'D') then ! daily
write(ncfile,'(a,a,i4.4,a,i2.2,a,i2.2,a,a)') &
history_file(1:lenstr(history_file))//trim(cstream), &
'.',iyear,'-',imonth,'-',iday,'.',suffix
elseif (histfreq(ns) == 'h'.or.histfreq(ns) == 'H') then ! hourly
write(ncfile,'(a,a,i2.2,a,i4.4,a,i2.2,a,i2.2,a,i5.5,a,a)') &
history_file(1:lenstr(history_file))//trim(cstream),'_', &
histfreq_n(ns),'h.',iyear,'-',imonth,'-',iday,'-',msec,'.',suffix
elseif (histfreq(ns) == 'm'.or.histfreq(ns) == 'M') then ! monthly
write(ncfile,'(a,a,i4.4,a,i2.2,a,a)') &
history_file(1:lenstr(history_file))//trim(cstream),'.', &
iyear,'-',imonth,'.',suffix
elseif (histfreq(ns) == 'y'.or.histfreq(ns) == 'Y') then ! yearly
write(ncfile,'(a,a,i4.4,a,a)') &
history_file(1:lenstr(history_file))//trim(cstream),'.', &
iyear,'.',suffix
endif
if (hist_avg) then ! write averaged data
if (histfreq(ns) == '1' .and. histfreq_n(ns) == 1) then ! timestep
write(ncfile,'(a,a,i4.4,a,i2.2,a,i2.2,a,i5.5,a,a)') &
history_file(1:lenstr(history_file))//trim(cstream),'_inst.', &
iyear,'-',imonth,'-',iday,'-',msec,'.',trim(suffix)
Comment on lines +704 to +707
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I understand why we need a separate if branch for this first case. Is it not covered by the

else ! instantaneous

line at the end of the subroutine ?

I guess the idea is that someone might have hist_avg = true and histfreq =1 and histfreq_n = 1, and in this case we ignore hist_avg (since there is nothing to average) and write instantaneous output instead ?

I'm wondering if it might be more useful to catch that early in ice_init and reset hist_avg to false in that case.. That would simplify the code here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With multiple history streams and a single hist_avg variable, we do not want to change hist_avg to false if one of the streams has histfreq='1' and histfreq_n=1. A future goal would be to make hist_avg an array as Dave suggested then we have more options.

elseif (histfreq(ns) == '1' .and. histfreq_n(ns) > 1) then ! timestep
write(ncfile,'(a,a,i4.4,a,i2.2,a,i2.2,a,i5.5,a,a)') &
history_file(1:lenstr(history_file))//trim(cstream),'.', &
iyear,'-',imonth,'-',iday,'-',msec,'.',trim(suffix)
elseif (histfreq(ns) == 'h'.or.histfreq(ns) == 'H') then ! hourly
write(ncfile,'(a,a,i2.2,a,i4.4,a,i2.2,a,i2.2,a,i5.5,a,a)') &
history_file(1:lenstr(history_file))//trim(cstream),'_', &
histfreq_n(ns),'h.',iyear,'-',imonth,'-',iday,'-',msec,'.',trim(suffix)
elseif (histfreq(ns) == 'd'.or.histfreq(ns) == 'D') then ! daily
write(ncfile,'(a,a,i4.4,a,i2.2,a,i2.2,a,a)') &
history_file(1:lenstr(history_file))//trim(cstream),'.', &
iyear,'-',imonth,'-',iday,'.',trim(suffix)
elseif (histfreq(ns) == 'm'.or.histfreq(ns) == 'M') then ! monthly
write(ncfile,'(a,a,i4.4,a,i2.2,a,a)') &
history_file(1:lenstr(history_file))//trim(cstream),'.', &
iyear,'-',imonth,'.',trim(suffix)
elseif (histfreq(ns) == 'y'.or.histfreq(ns) == 'Y') then ! yearly
write(ncfile,'(a,a,i4.4,a,a)') &
history_file(1:lenstr(history_file))//trim(cstream),'.', &
iyear,'.',trim(suffix)
endif

else ! instantaneous
write(ncfile,'(a,a,i4.4,a,i2.2,a,i2.2,a,i5.5,a,a)') &
history_file(1:lenstr(history_file))//trim(cstream),'_inst.', &
iyear,'-',imonth,'-',iday,'-',msec,'.',trim(suffix)
endif

else ! instantaneous with histfreq > dt
write(ncfile,'(a,a,i4.4,a,i2.2,a,i2.2,a,i5.5,a,a)') &
history_file(1:lenstr(history_file)),'_inst.', &
iyear,'-',imonth,'-',iday,'-',msec,'.',suffix
endif
endif

end subroutine construct_filename
Expand Down
30 changes: 18 additions & 12 deletions cicecore/cicedynB/infrastructure/io/io_netcdf/ice_history_write.F90
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ subroutine ice_write_hist (ns)
use ice_blocks, only: nx_block, ny_block
use ice_broadcast, only: broadcast_scalar
use ice_calendar, only: msec, timesecs, idate, idate0, write_ic, &
histfreq, days_per_year, use_leap_years, dayyr, &
year_init, month_init, day_init
histfreq, histfreq_n, days_per_year, use_leap_years, dayyr, &
hh_init, mm_init, ss_init
use ice_communicate, only: my_task, master_task
use ice_domain, only: distrb_info
use ice_domain_size, only: nx_global, ny_global, max_nstrm, max_blocks
Expand Down Expand Up @@ -205,7 +205,6 @@ subroutine ice_write_hist (ns)
! define coordinate variables
!-----------------------------------------------------------------

!sgl status = nf90_def_var(ncid,'time',nf90_float,timid,varid)
status = nf90_def_var(ncid,'time',nf90_double,timid,varid)
if (status /= nf90_noerr) call abort_ice(subname// &
'ERROR: defining var time')
Expand All @@ -215,8 +214,9 @@ subroutine ice_write_hist (ns)
'ice Error: time long_name')

write(cdate,'(i8.8)') idate0
write(title,'(a,a,a,a,a,a,a,a)') 'days since ', &
cdate(1:4),'-',cdate(5:6),'-',cdate(7:8),' 00:00:00'
write(title,'(a,a4,a1,a2,a1,a2,a1,i2.2,a1,i2.2,a1,i2.2)') 'days since ', &
cdate(1:4),'-',cdate(5:6),'-',cdate(7:8),' ', &
hh_init,':',mm_init,':',ss_init
status = nf90_put_att(ncid,varid,'units',title)
if (status /= nf90_noerr) call abort_ice(subname// &
'ERROR: time units')
Expand Down Expand Up @@ -258,8 +258,9 @@ subroutine ice_write_hist (ns)
if (status /= nf90_noerr) call abort_ice(subname// &
'ERROR: time_bounds long_name')
write(cdate,'(i8.8)') idate0
write(title,'(a,a,a,a,a,a,a,a)') 'days since ', &
cdate(1:4),'-',cdate(5:6),'-',cdate(7:8),' 00:00:00'
write(title,'(a,a4,a1,a2,a1,a2,a1,i2.2,a1,i2.2,a1,i2.2)') 'days since ', &
cdate(1:4),'-',cdate(5:6),'-',cdate(7:8),' ', &
hh_init,':',mm_init,':',ss_init
status = nf90_put_att(ncid,varid,'units',title)
if (status /= nf90_noerr) call abort_ice(subname// &
'ERROR: time_bounds units')
Expand Down Expand Up @@ -575,7 +576,8 @@ subroutine ice_write_hist (ns)
endif
endif

if (histfreq(ns) == '1' .or. .not. hist_avg &
if ((histfreq(ns) == '1' .and. histfreq_n(ns) == 1) &
.or..not. hist_avg &
.or. n==n_divu(ns) .or. n==n_shear(ns) & ! snapshots
.or. n==n_sig1(ns) .or. n==n_sig2(ns) &
.or. n==n_sigP(ns) .or. n==n_trsig(ns) &
Expand Down Expand Up @@ -640,7 +642,8 @@ subroutine ice_write_hist (ns)
'ERROR: defining cell methods for '//avail_hist_fields(n)%vname)
endif

if (histfreq(ns) == '1' .or. .not. hist_avg) then
if ((histfreq(ns) == '1' .and. histfreq_n(ns) == 1) &
.or..not. hist_avg) then
status = nf90_put_att(ncid,varid,'time_rep','instantaneous')
else
status = nf90_put_att(ncid,varid,'time_rep','averaged')
Expand Down Expand Up @@ -881,7 +884,8 @@ subroutine ice_write_hist (ns)
'ERROR: defining cell methods for '//avail_hist_fields(n)%vname)
endif

if (histfreq(ns) == '1' .or. .not. hist_avg) then
if ((histfreq(ns) == '1' .and. histfreq_n(ns) == 1) &
.or..not. hist_avg) then
status = nf90_put_att(ncid,varid,'time_rep','instantaneous')
else
status = nf90_put_att(ncid,varid,'time_rep','averaged')
Expand Down Expand Up @@ -942,7 +946,8 @@ subroutine ice_write_hist (ns)
'ERROR: defining cell methods for '//avail_hist_fields(n)%vname)
endif

if (histfreq(ns) == '1' .or. .not. hist_avg) then
if ((histfreq(ns) == '1' .and. histfreq_n(ns) == 1) &
.or..not. hist_avg) then
status = nf90_put_att(ncid,varid,'time_rep','instantaneous')
else
status = nf90_put_att(ncid,varid,'time_rep','averaged')
Expand Down Expand Up @@ -1003,7 +1008,8 @@ subroutine ice_write_hist (ns)
'ERROR: defining cell methods for '//avail_hist_fields(n)%vname)
endif

if (histfreq(ns) == '1' .or. .not. hist_avg) then
if ((histfreq(ns) == '1' .and. histfreq_n(ns) == 1) &
.or..not. hist_avg) then
status = nf90_put_att(ncid,varid,'time_rep','instantaneous')
else
status = nf90_put_att(ncid,varid,'time_rep','averaged')
Expand Down
Loading