Add snowtable implementation #3
Conversation
There was a problem hiding this comment.
This all looks good to me, though I have some questions below. For CICE, I think we should only have the 'file' option, not 'snicar'. For Icepack maybe we should keep both in the code but only offer 'file' officially, so that MPAS can use the 'snicar' approach if necessary.
| @@ -1,7 +1,17 @@ | |||
| tr_snow = .true. | |||
| snwgrain = .true. | |||
There was a problem hiding this comment.
snwgrain and use_smliq_pnd should not be set here - my mistake
There was a problem hiding this comment.
Both set_nml.nsw30percent and set_nml.nswITDrdg control wind redistribution of snow, which is fundamentally a separate process from the snow grain metamorphism. What I had in mind is to be able to set the processes individually (with others turned off) as -s snw30percent or -s snwITDrdg, or together as e.g. -s snwITDrdg,snwgrain. So I don't think we should have snw_aging_table etc in the snow redistribution options, but it should be in snwgrain. In the case where we want to run both, that should set snw_aging_table appropriately regardless of the order -s snwITDrdg,snwgrain or -s snwgrain,snwITDrdg, right?
| tr_snow = .true. | ||
| snwgrain = .true. | ||
| tr_snow = .true. | ||
| snwredist = 'ITDrdg' |
There was a problem hiding this comment.
snwredist should probably not be set here
There was a problem hiding this comment.
I leave it to you to review and update the snw set_nml settings. I updated the defaults, so all the snw flags were basically off. So the set_nml.snw* files needed to be updated.
There was a problem hiding this comment.
I will change the option file appropriately.
| deallocate(snowage_drdt0) | ||
|
|
||
| else | ||
| ! read everything and pass it in |
There was a problem hiding this comment.
I'm in favor of just reading everything in, if it's all available in the file anyhow. What is different in the file from the coded version? I'm wondering if I don't have the most up-to-date file.
There was a problem hiding this comment.
See the icepack PR. I set the mode to "file" in the set_nml files because I agree it's the right way to do it. Also, I have not implemented any table reading in Icepack. That functionality could be ported there, but I thought Icepack was just going to use the "test" data.
There was a problem hiding this comment.
Icepack will only use the 'test' data. If we have both 'snicar' and 'file' in Icepack and remove 'snicar' from CICE, then 'snicar' in Icepack won't get tested. I'm in favor of removing it completely. Will wait for N's response.
PR checklist
Add snowtable implementation
apcraig
Tested on cheyenne intel with Icepack isnow1tc2 mods. All results pass but not bit-for-bit with the current consortium master. https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#8a90b84895c3a42806db0345a4183e22715ee066. Answers here should not be changed relative to current snow1 branch.
The new implementation has the following options
snw_aging_table="test", internally sets the snow table to be the 5x5x1 dataset.
snw_aging_table="snicar", reads the tau, kappa, and drdt0 3D fields from the snw_filename file, reading snw_tau_fname, snw_kappa_fname, and snw_drdt0_fname fields. The one dimensional fields that support the table lookup are hardwired in icepack (and not totally consistent with the snicar data).
snw_aging_table="file", read the tau, kappa, and drdt0 3D fields from the snw_filename file, reading snw_tau_fname, snw_kappa_fname, and snw_drdt0_fname fields. In addition, the 1D rhos, Tgrd, and T fields are read from the same file using the snw_rhos_fname, snw_Tgrd_fname, and snw_T_fname field names.
The icepack table lookup only supports tables that have 1D regularly spaces dimensions. A place holder for non-regularly dimensions is in Icepack, but not yet developed. Many checks were added to Icepack to ensure the data sizes and regularity are consistent with the implementation.
Minimal documentation on the namelist are added, but more may be needed before merging with master.