Update snow table implementation and add SNICAR SSP Tables

[apcraig]

Update snow table implementation and add SNICAR SSP Tables

[apcraig]

I will rebase tomorrow with your latest changes. This updates the snow table initialization and adds various SNICAR SSP table options including reading in a file in the icepack driver and passing it into Icepack. I have tested the various options, the SNICAR science aborts during timestepping, but the tables seem to read fine for default (no table), snicartest (5x5 table) and snicar (5x1471) tables. I have also verified that the default setup with the new snow table initialization is also bit-for-bit for a few tests I've run. We can catch up more tomorrow depending whether what you want me to merge and if you want any changes.

[eclare108213]

AWESOME! extremely helpful.

[eclare108213]

so far icepack_init_radiation is only needed for dEdd, so this sectioncould be called inside the endif statement immediately above

[apcraig]

icepack_init_radiation calls the data_dEdd_3band which is called all the time right now. My preference would be that the icepack_init_radiation call be always called and then the if tests be inside that subroutine. That provides extensibility in the future for the icepack_init_radiation call if other stuff needs to be initialized for non-dEdd options. What I'll do is add an "if dEdd" around the data_dEdd_3band call in the subroutine and check that doesn't break anything.

[eclare108213]

good catch

[eclare108213]

should all of this table allocation code have an 'if (dEdd_snicar)' wrap around it?

[apcraig]

It probably doesn't matter, but I would say no. I think anytime a user passes something into icepack_parameters, it should be set that way inside icepack, even if it's not needed. Adding "if" will just increase complexity and if something is setup improperly, will be harder to debug.

[eclare108213]

change the comment (dr) and (df)

[apcraig]

Will do.

[eclare108213]

What needs to be done to fix the conflicts? I'll work on this, this afternoon, and would like to update to this version before I start.

[apcraig]

Let me rebase my PR and I'll also address your comments. If you can give me a couple hours, I should have it all sorted out, maybe sooner.

[eclare108213]

ok, thanks

[apcraig]

I am running a small test suite now, but I think it should be OK. There are a number of things to do

• The SSP array names used to set/read data (i.e. ssp_snwextdr) are not consistent with the names used in the dEdd subroutine (i.e. ext_cff_mss_ice_drc). This needs to be sorted out.
• I'm unclear about how the snow grain bands are related in the "snow" and "SNICAR SSP" codes. Both are somehow "SNICAR" data too? I think we should also rename "nmbrad" in icepack_shortwave to "nmbrad_snw" to more clearly differentiate from "nmbrad_snicar".
• We may have a significant issue with the SNICAR SSP table data. In both the input file and the "test" data, the array order is (nSnowGrainRadiusSNICAR, nSpectralIntervalsSNICAR). In the code, the order of those arrays when used is reversed. We allocate the data in the order specified by the code, but we read it in with the arrays flipped. The million dollar question is why anyone would ever implement arrays where the data and the code had the arrays flipped!!!! A few things
  • my recommendation is that we swap the order in the code, but this might be inconsistent with what's done in other dEdd SNICAR implemenations.
  • can we confirm that E3SM somehow flips the order of the data when it's read in?
  • this is almost certainly causing science problems, we'll need to sort it out first.

[apcraig]

Test results are below. baseline codes pass bit-for-bit except for intel with optimization. debug option is bit-for-bit for intel. All the snicar cases fail with a "Picard solver non-convergence". This is expected. I will run a fuller test suite to check bit-for-bit with the current baseline.

PASS cheyenne_intel_smoke_col_1x1_diag1_run1year build
PASS cheyenne_intel_smoke_col_1x1_diag1_run1year run
PASS cheyenne_intel_smoke_col_1x1_diag1_run1year test
FAIL cheyenne_intel_smoke_col_1x1_diag1_run1year compare icepack.e82e3d105b.220820-085005 different-data
#---
PASS cheyenne_intel_smoke_col_1x1_debug_run1year build
PASS cheyenne_intel_smoke_col_1x1_debug_run1year run
PASS cheyenne_intel_smoke_col_1x1_debug_run1year test
PASS cheyenne_intel_smoke_col_1x1_debug_run1year compare icepack.e82e3d105b.220820-085005
#---
PASS cheyenne_intel_smoke_col_1x1_diag1_run1year_snicartest build
FAIL cheyenne_intel_smoke_col_1x1_diag1_run1year_snicartest run
FAIL cheyenne_intel_smoke_col_1x1_diag1_run1year_snicartest test
MISS cheyenne_intel_smoke_col_1x1_diag1_run1year_snicartest compare icepack.e82e3d105b.220820-085005 missing-data
#---
PASS cheyenne_intel_smoke_col_1x1_debug_run1year_snicartest build
FAIL cheyenne_intel_smoke_col_1x1_debug_run1year_snicartest run
FAIL cheyenne_intel_smoke_col_1x1_debug_run1year_snicartest test
MISS cheyenne_intel_smoke_col_1x1_debug_run1year_snicartest compare icepack.e82e3d105b.220820-085005 missing-data
#---
PASS cheyenne_intel_smoke_col_1x1_diag1_run1year_snicar build
FAIL cheyenne_intel_smoke_col_1x1_diag1_run1year_snicar run
FAIL cheyenne_intel_smoke_col_1x1_diag1_run1year_snicar test
MISS cheyenne_intel_smoke_col_1x1_diag1_run1year_snicar compare icepack.e82e3d105b.220820-085005 missing-data
#---
PASS cheyenne_intel_smoke_col_1x1_debug_run1year_snicar build
FAIL cheyenne_intel_smoke_col_1x1_debug_run1year_snicar run
FAIL cheyenne_intel_smoke_col_1x1_debug_run1year_snicar test
MISS cheyenne_intel_smoke_col_1x1_debug_run1year_snicar compare icepack.e82e3d105b.220820-085005 missing-data
#---
PASS cheyenne_gnu_smoke_col_1x1_diag1_run1year build
PASS cheyenne_gnu_smoke_col_1x1_diag1_run1year run
PASS cheyenne_gnu_smoke_col_1x1_diag1_run1year test
PASS cheyenne_gnu_smoke_col_1x1_diag1_run1year compare icepack.e82e3d105b.220820-085005
#---
PASS cheyenne_gnu_smoke_col_1x1_debug_run1year build
PASS cheyenne_gnu_smoke_col_1x1_debug_run1year run
PASS cheyenne_gnu_smoke_col_1x1_debug_run1year test
PASS cheyenne_gnu_smoke_col_1x1_debug_run1year compare icepack.e82e3d105b.220820-085005
#---
PASS cheyenne_gnu_smoke_col_1x1_diag1_run1year_snicartest build
FAIL cheyenne_gnu_smoke_col_1x1_diag1_run1year_snicartest run
FAIL cheyenne_gnu_smoke_col_1x1_diag1_run1year_snicartest test
MISS cheyenne_gnu_smoke_col_1x1_diag1_run1year_snicartest compare icepack.e82e3d105b.220820-085005 missing-data
#---
PASS cheyenne_gnu_smoke_col_1x1_debug_run1year_snicartest build
FAIL cheyenne_gnu_smoke_col_1x1_debug_run1year_snicartest run
FAIL cheyenne_gnu_smoke_col_1x1_debug_run1year_snicartest test
MISS cheyenne_gnu_smoke_col_1x1_debug_run1year_snicartest compare icepack.e82e3d105b.220820-085005 missing-data
#---
PASS cheyenne_gnu_smoke_col_1x1_diag1_run1year_snicar build
FAIL cheyenne_gnu_smoke_col_1x1_diag1_run1year_snicar run
FAIL cheyenne_gnu_smoke_col_1x1_diag1_run1year_snicar test
MISS cheyenne_gnu_smoke_col_1x1_diag1_run1year_snicar compare icepack.e82e3d105b.220820-085005 missing-data
#---
PASS cheyenne_gnu_smoke_col_1x1_debug_run1year_snicar build
FAIL cheyenne_gnu_smoke_col_1x1_debug_run1year_snicar run
FAIL cheyenne_gnu_smoke_col_1x1_debug_run1year_snicar test
MISS cheyenne_gnu_smoke_col_1x1_debug_run1year_snicar compare icepack.e82e3d105b.220820-085005 missing-data
#---
PASS cheyenne_pgi_smoke_col_1x1_diag1_run1year build
PASS cheyenne_pgi_smoke_col_1x1_diag1_run1year run
PASS cheyenne_pgi_smoke_col_1x1_diag1_run1year test
PASS cheyenne_pgi_smoke_col_1x1_diag1_run1year compare icepack.e82e3d105b.220820-085005
#---
PASS cheyenne_pgi_smoke_col_1x1_debug_run1year build
PASS cheyenne_pgi_smoke_col_1x1_debug_run1year run
PASS cheyenne_pgi_smoke_col_1x1_debug_run1year test
PASS cheyenne_pgi_smoke_col_1x1_debug_run1year compare icepack.e82e3d105b.220820-085005
#---
PASS cheyenne_pgi_smoke_col_1x1_diag1_run1year_snicartest build
FAIL cheyenne_pgi_smoke_col_1x1_diag1_run1year_snicartest run
FAIL cheyenne_pgi_smoke_col_1x1_diag1_run1year_snicartest test
MISS cheyenne_pgi_smoke_col_1x1_diag1_run1year_snicartest compare icepack.e82e3d105b.220820-085005 missing-data
#---
PASS cheyenne_pgi_smoke_col_1x1_debug_run1year_snicartest build
FAIL cheyenne_pgi_smoke_col_1x1_debug_run1year_snicartest run
FAIL cheyenne_pgi_smoke_col_1x1_debug_run1year_snicartest test
MISS cheyenne_pgi_smoke_col_1x1_debug_run1year_snicartest compare icepack.e82e3d105b.220820-085005 missing-data
#---
PASS cheyenne_pgi_smoke_col_1x1_diag1_run1year_snicar build
FAIL cheyenne_pgi_smoke_col_1x1_diag1_run1year_snicar run
FAIL cheyenne_pgi_smoke_col_1x1_diag1_run1year_snicar test
MISS cheyenne_pgi_smoke_col_1x1_diag1_run1year_snicar compare icepack.e82e3d105b.220820-085005 missing-data
#---
PASS cheyenne_pgi_smoke_col_1x1_debug_run1year_snicar build
FAIL cheyenne_pgi_smoke_col_1x1_debug_run1year_snicar run
FAIL cheyenne_pgi_smoke_col_1x1_debug_run1year_snicar test
MISS cheyenne_pgi_smoke_col_1x1_debug_run1year_snicar compare icepack.e82e3d105b.220820-085005 missing-data

[eclare108213]

I am running a small test suite now, but I think it should be OK. There are a number of things to do

• The SSP array names used to set/read data (i.e. ssp_snwextdr) are not consistent with the names used in the dEdd subroutine (i.e. ext_cff_mss_ice_drc). This needs to be sorted out.

It's on my list. I'm keeping the lower level names from the MPAS-SI implementation for now, for my own sanity, but plan to change them to the higher-level, more intuitive names.

• I'm unclear about how the snow grain bands are related in the "snow" and "SNICAR SSP" codes. Both are somehow "SNICAR" data too? I think we should also rename "nmbrad" in icepack_shortwave to "nmbrad_snw" to more clearly differentiate from "nmbrad_snicar".

I can look at this. Haven't paid much attention to the snow model portion of the code changes, yet.

• We may have a significant issue with the SNICAR SSP table data. In both the input file and the "test" data, the array order is (nSnowGrainRadiusSNICAR, nSpectralIntervalsSNICAR). In the code, the order of those arrays when used is reversed. We allocate the data in the order specified by the code, but we read it in with the arrays flipped. The million dollar question is why anyone would ever implement arrays where the data and the code had the arrays flipped!!!! A few things

  • my recommendation is that we swap the order in the code, but this might be inconsistent with what's done in other dEdd SNICAR implemenations.
  • can we confirm that E3SM somehow flips the order of the data when it's read in?
  • this is almost certainly causing science problems, we'll need to sort it out first.

Eeeek! This might be my mistake in transferring code from MPAS-SI to Icepack -- I'll double check. If I remember correctly, the order listed in netCDF headers is backwards from that in the code. Is that true, and if so, is that the problem?

[eclare108213]

Is this ready to merge, and then I can continue working on all these issues?

[apcraig]

I tried to figure out whether the netcdf dump was definitely giving me information in "C" order or "Fortran" order and to be honest, it's a little unclear. I looked into that and wasn't sure. That could definitely be it and maybe it is. I'll interrogate the data after it's read into the code to confirm. If that's the case, then we should be OK. But there is still a question where the "test" table data says,

!
! ext_cff_mss_ice_drc
!
iceMassExtinctionCrossSectionDirect = reshape((/ &
46.27374983, 24.70286257,  6.54918455,  2.6035624, &
...
47.48598349, 25.14100194,  6.59708024,  2.61372576,  1.19897351 /),&
(/nSnowGrainRadiusSNICAR,nSpectralIntervalsSNICAR/))

which is backwards. But since nmbrad=nspint=5 in this case, it won't affect the table. The question is whether the data order is correct or whether the index order is correct in the table. If the data order is correct, then we can just flip the indices (for clarity) and leave the data alone. If the data is backwards, then we need to flip the data around in the data statement. Partly, I assumed the netcdf and "test" data were done in the same way which is why I raised the flag in general. But it could just be that the "test" data has the indices reversed, playing no role in the science.

[apcraig]

You can merge and I'll continue to test.

[eclare108213]

In the original code:
ks = ext_cff_mss_ice_drc(ns,nr)
ws = ss_alb_ice_drc(ns,nr)
gs = asm_prm_ice_drc(ns,nr)
ns = index for spectral bands, nr = index for radii

so the reshape functions in the code should be to (/nSnowGrainRadiusSNICAR,nSpectralIntervalsSNICAR/)
which might mean flipping those 5x5 data arrays

I think you already know this, I'm just catching up