Clean up Icepack interfaces

[apcraig]

PR checklist

• Short (1 sentence) summary of your PR:
  Clean up Interfaces, remove redundant arguments

• Developer(s):
  apcraig

• Suggest PR reviewers from list in the column to the right.

• Please copy the PR test results link or provide a summary of testing completed below.
  Full test suite run on Derecho with intel, cray, and gnu. This is bit-for-bit except many of the bgc configurations still DO NOT run correctly/successfully in standalone mode. There is still work to be done to make the code more robust.

• How much do the PR code changes differ from the unmodified code?

  • bit for bit
  • different at roundoff level
  • more substantial

• Does this PR create or have dependencies on CICE or any other models?

  • Yes
  • No

• Does this PR add any new test cases?

  • Yes
  • No

• Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.)

  • Yes
  • No, does the documentation need to be updated at a later time?
    • Yes
    • No

• Please provide any additional information or relevant details below:

  Migrate bgrid, cgrid, igrid, icgrid, swgrid to Icepack variables,
  remove them from the driver and interfaces.

  Remove redundant Icepack interface arguments set in parameters and tracers

  Update merge_fluxes to make all merges optional

  Update derecho inputdata (currently on main)

  Update conda settings (currently on main)

[apcraig]

These are the set of changes I want to be included in the bgc merge into the Consortium. This largely just removes arguments from interfaces that don't need to be there.

These changes will require the E3SM and CICE calls to Icepack to be updated. Mostly what needs to happen is that extra arguments are removed from those calls, so it should be easy to do. These changes are all bit-for-bit and don't impact the model runs.

Separately, I will continue to try to expand use of optional arguments and will do that as a separate PR if I find anything.

[apcraig]

I want to point out another thing I just noticed that I think has to be fixed. In the icepack parameters, there is a new input with the bgc, dictype_1. That is similar to doctype_l. But dictype has a "number one" and doctype has a "lower case L". I think it should be rename dictype_l with lower case L?

Happy to fix that if everyone agrees.

[apcraig]

Just a quick follow up. It's very confusing actually. We have

     ratio_Fe2DOC_l     = 0.033_dbl_kind , & ! Fe to C of DOC (nmol/umol) lipids
     f_doc_l            = 0.5_dbl_kind   , &
     f_exude_l          = c1 , &
     k_bac_l            = 0.03_dbl_kind  , &
     doctype_l          = c0 , & !

which all have an undercase then lower case L. Then we have

     dictype_1          = -1.0_dbl_kind  , & ! DIC
     fedtype_1          = c0 , & ! Iron
     feptype_1          = 0.5_dbl_kind   , & !

which all have an undercase and then number 1. In the code, the 1 and L look very, very similar. Is there a reason some are "one" and some are "L"? It would be really good to either unify to "one" or "L" or to use some other nomenclature that is not a "one" or an "L" for one or the other. Let me know if there is something I should do.

[apcraig]

I have taken the latest version of the E3SM Icepack changes (including this PR) and merged it into a sandbox of the Consortium main Icepack and CICE codes. I then ran full test suites on derecho.

For Icepack, everything is bit-for-bit except the bgc. The bgc results still have several problems and results vary depending on compiler,

PASS derecho_intel_smoke_col_1x1_bgcispol_debug run
PASS derecho_intel_smoke_col_1x1_bgcnice_debug run
FAIL derecho_intel_smoke_col_1x1_bgcsklnice_debug run
PEND derecho_intel_restart_col_1x1_bgcispol run
PASS derecho_intel_restart_col_1x1_bgcnice run 
PASS derecho_intel_restart_col_1x1_bgcsklnice run 
FAIL derecho_cray_smoke_col_1x1_bgcispol_debug run
FAIL derecho_cray_smoke_col_1x1_bgcnice_debug run
PASS derecho_cray_smoke_col_1x1_bgcsklnice_debug run
FAIL derecho_cray_restart_col_1x1_bgcispol run
FAIL derecho_cray_restart_col_1x1_bgcnice run
PASS derecho_cray_restart_col_1x1_bgcsklnice run 
FAIL derecho_gnu_smoke_col_1x1_bgcispol_debug run
FAIL derecho_gnu_smoke_col_1x1_bgcnice_debug run
FAIL derecho_gnu_smoke_col_1x1_bgcsklnice_debug run
PASS derecho_gnu_restart_col_1x1_bgcispol run 
PASS derecho_gnu_restart_col_1x1_bgcnice run 
PASS derecho_gnu_restart_col_1x1_bgcsklnice run 
PASS derecho_nvhpc_smoke_col_1x1_bgcispol_debug run
PASS derecho_nvhpc_smoke_col_1x1_bgcnice_debug run
PASS derecho_nvhpc_smoke_col_1x1_bgcsklnice_debug run
PASS derecho_nvhpc_restart_col_1x1_bgcispol run 
PASS derecho_nvhpc_restart_col_1x1_bgcnice run 
PASS derecho_nvhpc_restart_col_1x1_bgcsklnice run 
PASS derecho_intelclassic_smoke_col_1x1_bgcispol_debug run
PASS derecho_intelclassic_smoke_col_1x1_bgcnice_debug run
FAIL derecho_intelclassic_smoke_col_1x1_bgcsklnice_debug run
PEND derecho_intelclassic_restart_col_1x1_bgcispol run
PASS derecho_intelclassic_restart_col_1x1_bgcnice run 
PASS derecho_intelclassic_restart_col_1x1_bgcsklnice run 
PASS derecho_inteloneapi_smoke_col_1x1_bgcispol_debug run
PASS derecho_inteloneapi_smoke_col_1x1_bgcnice_debug run
FAIL derecho_inteloneapi_smoke_col_1x1_bgcsklnice_debug run
PEND derecho_inteloneapi_restart_col_1x1_bgcispol run
PASS derecho_inteloneapi_restart_col_1x1_bgcnice run 
PASS derecho_inteloneapi_restart_col_1x1_bgcsklnice run 

For CICE, I've just run the full test suite on Derecho with the Intel compiler. Again, all results are bit-for-bit with the current Consortium main code except the bgc results. For bgc, all the tests abort before they finish. It looks like I'm running into some issues with the Icepack warning package. I think @njeffery was having some similar problems. I will look into that. Certainly, the updated bgc generates a huge number of warning messages with CICE (algal_dyn conservations errors, etc). I'm also thinking the warning messages may not be fully thread proof yet and the huge number of messages produced by bgc now is leading to that failure. Again, I'll look into that further.

Overall, I think the changes we have are going OK. This PR seems to be robust in terms of updating the interfaces. I will work on the warning package problems, but on the SE side, I think we're in good shape. On the science side, we still have work to do to validate the bgc in standalone Icepack and CICE.

[njeffery]

Just a quick follow up. It's very confusing actually. We have

     ratio_Fe2DOC_l     = 0.033_dbl_kind , & ! Fe to C of DOC (nmol/umol) lipids
     f_doc_l            = 0.5_dbl_kind   , &
     f_exude_l          = c1 , &
     k_bac_l            = 0.03_dbl_kind  , &
     doctype_l          = c0 , & !

which all have an undercase then lower case L. Then we have

     dictype_1          = -1.0_dbl_kind  , & ! DIC
     fedtype_1          = c0 , & ! Iron
     feptype_1          = 0.5_dbl_kind   , & !

which all have an undercase and then number 1. In the code, the 1 and L look very, very similar. Is there a reason some are "one" and some are "L"? It would be really good to either unify to "one" or "L" or to use some other nomenclature that is not a "one" or an "L" for one or the other. Let me know if there is something I should do.

@apcraig : DOC have _l and _s . The 'l' stands for "lipids" and 's' for "saccharids". The other fields, dic/fe, are a single generic type.

[apcraig]

Thanks @njeffery for clarifying, that makes sense. In the source code, interfaces, and documentation, it's very difficult to tell the difference between the 1 and small L. It might be nice to come up with some other nomenclature for the 1 for those variables. But I leave that up to you. Even if we just removed the underscore, that would help, _1 -> 1. If you do decide you want something different, I'm happy to do the search and replace throughout Icepack for you and create the PR, just let me know.

I was working on the icepack warnings module today to try to make it thread safe. Made some progress, still some work to do. Will update the PR (or create a new one) when I have something ready. I think that will fix your issues with the warnings package as well. Thanks!

[njeffery]

@apcraig : we could use a capital? _L

[apcraig]

So you want _s (small s), _L (cap L) and _1 (one)? You're OK having lower case s and upper case L? I would say get rid of _1 (one), but there are already variables in use with that name. I'm open, could also leave it as is I guess. Happy to switch the _l to _L if that's what you want, just let me know. I would change the 5 variables noted above?

[njeffery]

@apcraig : It gets complicated. My vote is to leave it as is.

[apcraig]

OK, will leave as is.

[apcraig]

I just updated the PR to improve the icepack warnings package to make it more thread-safe. I ran the full icepack test suite with the E3SM Icepack + this PR and things look OK. Still running into lots of bgc issues, same as before the latest update.

Also testing these latest changes in the Consortium Icepack and CICE sandboxes. The updated icepack warnings module got rid of the CICE bgc seg faults associated with the warning package. I'm still getting several bgc failures. Most of the CICE bgc test cases time out due to the amount of output generated by Icepack. I'm going to turn off that excess Icepack bgc output and retest.

[eclare108213]

This PR has already been merged, but regarding the labeling, how about _lip and _sac?

[apcraig]

Happy to continue to think about the variable names. While the _l and _1 are confusing, the users will sort it out fairly quickly when their code don't compile. :)

@njeffery, thanks for updating your Icepack calls in E3SM, I know this was a bit of extra work, but think it's worthwhile in the long run.