Merge CICE-Consortium/Icepack main to E3SM-Project/Icepack main #35
Conversation
Co-authored-by: David Bailey <[email protected]> Co-authored-by: Elizabeth Hunke <[email protected]>
* Add .readthedocs.yaml Copy CICE's readthedocs configuration as of CICE-Consortium/CICE@06282a53 (Update version to 6.4.2 (#864), 2023-09-08). * doc: mention code variables relating to 'calc_dragio' At the end of the "Ocean" section, we mention that 'dragio' can be computed from 'iceruf_ocn' and 'thickness_ocn_layer1', but do not mention these code variables, nor the 'calc_dragio' setting used to activate that computation. Mention the code variables next to their mathematical symbol, for more clarity.
…ters (#465) All parameters of icepack_parameters::icepack_query_parameters are intent(out), so it does not make sense to recompute constants at the end of that subroutine since no constants are changed. This superfluous call dates back to the introduction of icepack_query_parameters (then called icepack_query_constants) in 7646f2a (Migrate icepack_constants out of the columnphysics/constants directory..., 2017-10-04). Remove that call.
* Remove older "_old" tfrz_options, added for backwards compatibility Modify tfrz_option logic in icepack_sea_freezing_temperature to trap unsupported values * Update Icepack set_nml settings, remove _old from tfrz_option
* Clarify documentation associated with namelist inputs Alphabetize cpl_frazil namelist documentation * Update documentation
* Update 5-band dEdd to support test data Add an interpolation method in icepack_shortwave.F90 for rsnw Refactor portions of icepack_shortwave to improve robustness Add snicar and snicartest test cases * Update the 5-band snicar shortwave rsnw_snicar_tab interpolation This changes answers but QC tests pass. The new implementation checks whether the rsnw_snicar_tab is an array of integer values with deltas of value 1 (and sets the rsnw_snicar_chk variable). If so, it uses a shortcut to find the rsnw_snicar_tab bounds for interpolation, otherwise it calls the shortwave_search routine. In testing, the shortcut did not change performance much, but that could change in the future. * Clean up debug output * Refactor the logic to control rsnw table interpolation in the shortwave. Set a character string, rsnw_datatype, at initialization.
…ation (#476) * correct ks units, clean up comments, big fix for BL99 enthalpy calculation * make it pretty
Update pull request template to ask for additional information about the PR. This information can and will be used during the squash merge process to produce more useful commit logs. Update Macros.conda_macos to remove ffpe-trap=invalid from debug flags. This flag causes nf90_create to fail with the latest version of the conda environment. This happens when debug flags are on and a test with netcdf is turned on. I believe this is a compiler error.
* Add gnuplot to the conda environment This fix allows the timeseries.csh script to be used for Icepack standalone runs.
|
I am testing this using the E3SM-Polar-Developer script. |
|
Running QC test on the following directories: |
|
Baseline E3SM: E3SM with updated Icepack |
|
Here are the stats comparing the 5-year QC runs. |
There was a problem hiding this comment.
We have worked further on the high resolution version of E3SM and determined that the model is also crashing with a data sea ice model, thereby eliminating anything in this PR as a potential cause of model instability. In consultation with the HR team and with @eclare108213, I would like to suggest we close this PR so as not to complicate @njeffery 's work that would automatically see this change brought into E3SM as part of the BGC tag.
|
The contents of this PR will likely NOT be brought in with @njeffery's BGC mods. I think it's best to keep them separate, since they are both somewhat large collections of changes and their intent is different. The argument for not merging this PR yet is so that it's easier to compare Nicole's code / simulations to existing fully coupled (water cycle) runs. If the control runs will not be BFB due to other changes/bug fixes, then I we could go ahead with this PR. Then @njeffery would need to rebase and retest -- that's extra work that we'd prefer to avoid. Are non-BFB changes going into master now? |
|
Also, there really isn't any risk/cost to merge this. If the plan is to keep the E3SM-Project/Icepack main synced up with the Consortium/Icepack main, this PR is just doing that. We could probably create a github action that would do it automatically once a week and it would have no impact on any other development or plans. Merging this PR has no impact on the bgc branch or whether bgc is updated to the current main. That is a totally independent step. It also has no impact on the Icepack hash that E3SM points to. That hash can be on any branch. At some points, it could be a hash from main, but never has to be. All that assumes the plan is for the E3SM-Project/Icepack main to match the Consortium/Icepack main exactly always and for that update to happen periodically. |
|
That's true, E3SM will not point to this new hash. But I think this branch (main) is where we had planned to force-push changes from the Consortium in order to keep the history clean, so I don't think we would actually merge this PR anyhow. Mostly I don't want to confuse the order in which things happen, relative to the BGC merge. If we're going to make these changes in E3SM first, then let's do it following the established process, which I think means force-pushing to E3SM Icepack main and then opening another PR into E3SM itself, linking the new Icepack hash, and moving the documentation above into that PR. But that's harder for Nicole to deal with, so I'm waiting. It's good to know that my tests above succeeded cleanly. I do not know whether Rob et al will want us to update the Icepack link in E3SM separately for the resync and for the BGC, or if we could PR them together - my guess is that they'll need to be done separately. |
|
I'm not sure I understand the plan. The current E3SM Icepack main is up to date with Consortium from a few months ago. I think we forced pushed then, so the history is identical, just E3SM is a bit behind. This PR just does this push from Consortium main to E3SM main, we don't need to force push, just don't squash merge and the mains will remain synced up with identical history. That is taken care of with this PR. As I said, we could probably automate this merge weekly if we wanted, don't need to force push as long as nobody pushed onto E3SM main when they shouldn't. For bgc, we may want or have to fix the history, but that would NOT have to happen from the head of main and what's on main wouldn't matter. We would find the appropriate hash to branch from on main (or elsewhere), create a branch from it, then merge the bgc changes onto it. When bgc is ready, the hash on that branch would be used in E3SM and then could be merged to Consortium main. We cannot and should not require the head of main in E3SM to be somehow associated with some current version of Icepack that's useful in E3SM. There may be hashes on main that are particularly useful wrt E3SM development, we can know/discover what those are quite easily. But the head of main should not be part of the equation of that. We should never be force pushing again in general. Only if someone commits something to main that shouldn't be there. We should probably clarify the process again, seems like there is some confusion. |
|
Okay, we need to sort through this. It's is already on the agenda for the Icepack meeting tomorrow: https://acme-climate.atlassian.net/wiki/spaces/ICE/pages/4254990453/2024-04-17+Icepack+Merge+Meeting+notes |
* add tutorial documentation * add tutorial * add tutorial * add tutorial * add tutorial * add tutorial * Minor clarifications and grammatical updates. --------- Co-authored-by: Elizabeth Hunke <[email protected]>
Update Copyright to 2024 in code and LICENSE file
Add run_testoutput script to generate release plots for Icepack
Documentation is being update based on the Consortium workshop in 2024. The "adding a tracer" section has been updated as has the tutorial section related to adding a fluff tracer. An example set of code diffs for the tutorial is also includes as reference.
Port to NCAR's cluster, casper using the HTC nodes using standard cpus. This will support testing and development on another piece of hardware other than derecho and izumi and provides a good option for smaller test cases. Added intel, inteloneapi, and gnu compiler options. The -check debug option is very sensitive in this version of Inteloneapi, so it has been turned off.
Add congel_freeze namelist and add 'one-step' option to the default 'two-step' option. Namelist flag congel_freeze chooses which formulation to use. The original is ‘two-step’, since only the mushy boundary moves in the first step and the phase change happens in the next step. Plante et al. (‘one-step’) moves the boundary and performs the phase change in a single time step. Maintain 'two-step' as default for now.
This is a significant update in the BGC including refactoring Icepack interfaces. Deprecate skl BGC but leave code alone for now hoping we get help from the community to validate the latest code. Add BGC parameters to icepack_parameters.F90 Update BGC physics, consistent with E3SM-Project/E3SM#6457. Remove redundant arguments in BGC interfaces. icepack_aerosol.F90 revised subroutine update_snow_bgc icepack_algae.F90 revised subroutine zbio revised subroutine z_biogeochemistry revised subroutine algal_dyn add subroutine bgc_carbon_sum icepack_brine.F90 revise subroutine prepare_hbrine revise subroutine update_hbrine icepack_mechred.F90 add mbio calculation to subroutine ridge_shift add flux_bio calculation to subroutine ridge_ice icepack_therm_itd.F90 update calculation of dvssl and dvint in subroutine lateral_melt icepack_zbgc.F90 lots of stuff icepack_zbgc_shared.F90 lots of stuff Remove redundant arguments in non-BGC interfaces. icepack_atmo.F90 icepack_fsd.F90 icepack_isotope.F90 icepack_itd.F90 icepack_meltpond_topo.F90 icepack_mushy_physics.F90 icepack_snow.F90 icepack_therm_bl99.F90 icepack_therm_mushy.F90 icepack_therm_shared.F90 icepack_therm_vertical.F90 icepack_tracers.F90 icepack_wavefracspec.F90 Generalize merge_fluxes to make all arguments optional Fix bug in subroutine snow_redist computation of hsn_new when nslyr=1 Update the Icepack driver consistent with Icepack interface changes Update zbgc initialization in the Icepack driver, move bgc parameter intiialization to icepack_init_parameters, update icepack_init_zbgc call. Update some calls to icepack_warnings_setabort to add file and line number. Update warning package to improve OpenMP robustness. Fixes potential race conditions that show up when lots of output is produced. Modified congel implementation in subroutine thickness_changes in icepack_therm_vertical.F90 to recover bit-for-bit results. New congel and new bgc implementation were bit-for-bit independently, but when combining the changes, the intel compiler (with -O2) introduces non bit-for-bit changes (roundoff). Update bgc namelist defaults and settings in icepack_in Update bgc tracer sizing in set_env files Update testing, remove skl tests, add zaero tests. --------- Co-authored-by: Elizabeth Hunke <[email protected]> Co-authored-by: David Bailey <[email protected]> Co-authored-by: Nicole Jeffery <[email protected]>
Update inteloneapi, cray, intel, nvhpc modules/compiler. Changes answers for inteloneapi and cray. Switch default Derecho queue from main to develop to support quicker turn around and lower costs. Develop is a shared queue. Error in the snow physics with the cray compiler needs further assessment, but suggest we merge this now as is. That error does not present itself in CICE with snow physics with the same cray compiler.
This is a simple PR created directly from the repositories.