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

Change "time" value for averaged quantities to midpoint of averaging period #869

Closed
wants to merge 7 commits into from

Conversation

peverwhee
Copy link
Collaborator

Overview

Per #159 - this PR changes the "time" variable in a history file with averaged (and/or min/max/stdev quantities) to be the midpoint of the averaging period (basically, the average of the time_bounds variable).

Side affects of the above/related changes in this PR (all implemented):

  • Instantaneous history files (avgflag_pertape = 'I') cannot have averaged fields due to the time mismatch
  • Averaged history files (avgflag_pertape /= 'I') cannot have instantaneous fields (same reason)
  • Changed 'time_bnds' variable name to 'time_bounds'
  • Ensured the 'cell_methods' metadata field always includes "time: XXXXX" (where XXXXX refers to the avgflag) for history fields so that it's always clear what operation is being done on/for that field

Tests performed locally

  • Tested out-of-the-box Kessler (h0 is instantaneous) - runs and time field is current time
  • Tested trying to add an average field to h0 - fails as expected
  • Tested adding an average tape (h1) - runs and time field is midpoint(s)
  • Tested adding an instantaneous field to h1 - fails as expected
  • Tested adding instantaneous field (":I") to h0 and average field (":M") to h1 - works as expected

Related to: ESCOMP/CTSM#2019

closes #159

@peverwhee peverwhee self-assigned this Aug 3, 2023
@peverwhee peverwhee added this to Open PRs - unassigned in CAM Development branch (cutting edge development) via automation Aug 3, 2023
@peverwhee
Copy link
Collaborator Author

Requesting @fvitt take a look at the updated WACCM use case namelists

@peverwhee peverwhee requested a review from fvitt August 4, 2023 22:57
Copy link

@fvitt fvitt left a comment

Choose a reason for hiding this comment

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

Some suggestions

Comment on lines 74 to 76
<avgflag_pertape> 'A', 'I', 'I', 'A', 'A', 'A', 'I' </avgflag_pertape>
<nhtfrq> 0, -1, -24, -24, -120, -24, -120 </nhtfrq>
<mfilt> 1, 24, 7, 7, 10, 365, 10 </mfilt>
Copy link

Choose a reason for hiding this comment

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

Suggested change
<avgflag_pertape> 'A', 'I', 'I', 'A', 'A', 'A', 'I' </avgflag_pertape>
<nhtfrq> 0, -1, -24, -24, -120, -24, -120 </nhtfrq>
<mfilt> 1, 24, 7, 7, 10, 365, 10 </mfilt>
<avgflag_pertape> 'A', 'I', 'I', 'A', 'A', 'A', 'A', 'I' </avgflag_pertape>
<nhtfrq> 0, -1, -24, -24, -120, -24, -24, -120 </nhtfrq>
<mfilt> 1, 24, 7, 7, 10, 365, 365, 73 </mfilt>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed, though I did not add the extra average history tape and instead changed the existing tape numbers in the finclx variables.

@@ -87,7 +87,7 @@
<!-- History Streams -->

<mfilt> 1, 30, 120, 240, 240, 480, 365, 73, 30 </mfilt>
Copy link

Choose a reason for hiding this comment

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

Suggested change
<mfilt> 1, 30, 120, 240, 240, 480, 365, 73, 30 </mfilt>
<mfilt> 1, 30, 120, 240, 240, 480, 365, 73, 73 </mfilt>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed.

@@ -67,7 +67,7 @@
<!-- History Streams -->

<mfilt> 1, 30, 120, 240, 240, 480, 365, 73, 30 </mfilt>
Copy link

Choose a reason for hiding this comment

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

Suggested change
<mfilt> 1, 30, 120, 240, 240, 480, 365, 73, 30 </mfilt>
<mfilt> 1, 30, 120, 240, 240, 480, 365, 73, 73 </mfilt>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed.

@@ -118,7 +118,7 @@
'BRO:B', 'NO3:B', 'DTCORE:B', 'DTV:B', 'TTGW:B','OMEGA:B' </fincl3>
<!-- Daily Average -->
<fincl4>
'PS', 'PSL', 'U', 'V', 'T', 'Z3', 'PHIS','FRONTGF:I', 'OMEGA'
'PS', 'PSL', 'U', 'V', 'T', 'Z3', 'PHIS','FRONTGF', 'OMEGA'
Copy link

Choose a reason for hiding this comment

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

Move 'FRONTGF' to fincl6

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

moved.

</fincl8>
</fincl7>
<!-- Instantaneous every 5 days -->
<fincl8>'FRONTGF'</fincl8>
Copy link

Choose a reason for hiding this comment

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

Set the 8th entry of nhtfrq to -120 and mfilt to 73 in this use case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed

@@ -146,8 +146,8 @@

<!-- History Files -->

<mfilt> 1, 5, 20, 40, 120, 240, 365, 73, 365 </mfilt>
<nhtfrq> 0, -24, -6, -3, -1, 1, -24,-120,-240 </nhtfrq>
<mfilt> 1, 5, 20, 40, 120, 240, 365, 73, 365 </mfilt>
Copy link

Choose a reason for hiding this comment

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

Suggested change
<mfilt> 1, 5, 20, 40, 120, 240, 365, 73, 365 </mfilt>
<mfilt> 1, 5, 20, 40, 120, 240, 365, 73, 73 </mfilt>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed.

@@ -42,8 +42,8 @@

<!-- History Files -->

<mfilt> 1, 5, 20, 40, 120, 240, 365, 73, 365 </mfilt>
<nhtfrq> 0, -24, -6, -3, -1, 1, -24,-120,-240 </nhtfrq>
<mfilt> 1, 5, 20, 40, 120, 240, 365, 73, 365 </mfilt>
Copy link

Choose a reason for hiding this comment

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

Suggested change
<mfilt> 1, 5, 20, 40, 120, 240, 365, 73, 365 </mfilt>
<mfilt> 1, 5, 20, 40, 120, 240, 365, 73, 73 </mfilt>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed.

@@ -136,7 +136,7 @@
'SAD_ICE', 'SAD_LNAT', 'SAD_SULFC', 'T', 'TREFHT', 'TTGW', 'U',
'UTGWORO', 'UTGWSPEC', 'V', 'VERT', 'VTGWORO', 'VTGWSPEC', 'Z3', 'O2_1S', 'O2_1D', 'NOX', 'NOY',
'CLOX', 'CLOY', 'BROX', 'BROY', 'TCLY', 'TOTH', 'QJOULE', 'UIONTEND', 'VIONTEND',
'DTCORE', 'CLDLIQ', 'CLDICE', 'CONCLD', 'FRONTGF:I', 'BUTGWSPEC', 'BTAUE', 'BTAUW', 'BTAUN', 'BTAUS', 'TAUE',
'DTCORE', 'CLDLIQ', 'CLDICE', 'CONCLD', 'FRONTGF', 'BUTGWSPEC', 'BTAUE', 'BTAUW', 'BTAUN', 'BTAUS', 'TAUE',
Copy link

Choose a reason for hiding this comment

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

Should move 'FRONTGF' to fincl4 which should be designated as 'I'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

moved and changed.

@@ -78,7 +78,7 @@
'Z3', 'T', 'TIon', 'TElec', 'e', 'U', 'V', 'OMEGA', 'UI', 'VI', 'WI', 'ElecColDens', 'PHIM2D', 'PS',
'PED_CONDUCTANCE', 'HALL_CONDUCTANCE', 'ED1', 'ED2', 'O', 'O2', 'H', 'NO', 'CO2', 'N', 'O1D',
'Op2P', 'Op2D', 'Op', 'Np', 'N2p', 'O2p', 'NOp', 'QJOULE', 'SIGMAHAL', 'SIGMAPED', 'SolIonRate_Tot',
'Z3GM', 'OpDens', 'EDens'
'Z3GM', 'OpDens', 'EDens', 'FRONTGF'
Copy link

Choose a reason for hiding this comment

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

'FRONTGF' should be moved to fincl4

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

moved and changed

@@ -114,7 +114,7 @@
'QRL_TOT', 'PSL', 'HNO3_STS', 'HNO3_NAT', 'HNO3_GAS', 'NO_Aircraft', 'NO_Lightning',
'QRS_AUR', 'QRS_CO2NIR', 'QRS_EUV', 'SAD_ICE', 'SAD_LNAT', 'SAD_SULFC', 'TREFHT',
'VERT', 'VTGWORO', 'VTGWSPEC', 'O2_1S', 'O2_1D', 'NOX', 'NOY', 'CLOX', 'CLOY', 'BROX', 'BROY',
'TCLY', 'TOTH', 'UIONTEND', 'VIONTEND', 'DTCORE', 'CLDLIQ', 'CLDICE', 'CONCLD', 'FRONTGF:I',
'TCLY', 'TOTH', 'UIONTEND', 'VIONTEND', 'DiTCORE', 'CLDLIQ', 'CLDICE', 'CONCLD',
Copy link

Choose a reason for hiding this comment

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

Suggested change
'TCLY', 'TOTH', 'UIONTEND', 'VIONTEND', 'DiTCORE', 'CLDLIQ', 'CLDICE', 'CONCLD',
'TCLY', 'TOTH', 'UIONTEND', 'VIONTEND', 'DTCORE', 'CLDLIQ', 'CLDICE', 'CONCLD',

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@peverwhee peverwhee requested a review from fvitt August 7, 2023 17:57
@peverwhee peverwhee marked this pull request as draft August 8, 2023 15:37
@peverwhee peverwhee closed this Oct 13, 2023
CAM Development branch (cutting edge development) automation moved this from Open PRs - unassigned to Completed tags Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

None yet

3 participants