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.

Sign up for GitHub

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

Jump to bottom

Update namelist implementation #382

Merged
merged 1 commit into from
Dec 24, 2021
Merged

Update namelist implementation #382

merged 1 commit into from
Dec 24, 2021

Conversation

apcraig
Copy link
Contributor

@apcraig apcraig commented Dec 22, 2021

PR checklist

  • Short (1 sentence) summary of your PR:
    Update namelist implementation to make group order arbitrary
  • 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 suites run on cheyenne and izumi, all pass, https://github.com/CICE-Consortium/Test-Results/wiki/icepack_by_hash_forks#3266c65a651e0ece6184a099fe4388b48d24f6c9 including NAG which was broken recently.
  • 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:

See also CICE-Consortium/CICE#676

  • Update namelist implementation to allow namelist groups to be read in any order.
  • Had to remove recent implementation to write bad namelist lines. That implementation conflicts with flexibility to read groups in any order. This really only affects NAG because other compilers automatically read thru the namelist file to find the group while NAG does not.

@apcraig
- Update namelist implementation to allow namelist groups to be read …
3266c65 
…in any order.

- Had to remove recent implementation to write bad namelist lines.  That implementation conflicts with flexibility to read groups in any order.  This really only affects NAG because other compilers automatically read thru the namelist file to find the group while NAG does not.
eclare108213
eclare108213 approved these changes Dec 22, 2021
View reviewed changes
Copy link
Contributor

@eclare108213 eclare108213 left a comment

Choose a reason for hiding this comment

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

Did you run a test with a rearranged ice_in file?

@apcraig
Copy link
Contributor Author

apcraig commented Dec 22, 2021

Did you run a test with a rearranged ice_in file?

I did better, I changed the order the groups were read in icepack, swapping tracer_nml and thermo_nml. Every test now tests the feature works. We should find quickly if the implementation fails on some machine/compiler now :)

@apcraig apcraig merged commit 19d9745 into CICE-Consortium:main Dec 24, 2021
lettie-roach pushed a commit to lettie-roach/Icepack that referenced this pull request Oct 18, 2022
@phil-blain
Add ECCC driver for coupling with NEMO, update 'hadgem3' driver (#605)
a63cc1c 
* drivers/hadgem3: add missing 'subname' and use existing 'subname's

* drivers/hadgem3/CICE_InitMod: update 'init_lvl' call

Add the required 'iblk' argument.

* drivers/hadgem3/CICE_RunMod: remove uneeded 'dt' arguments

The subroutines 'prep_radiation', 'zsal_diags', 'bgc_diags' and 'hbrine_diags'
do not take a 'dt' argument anymore, so remove it.

* drivers/hadgem3/CICE_RunMod: get 'Lsub' from Icepack

* drivers/hadgem3/CICE_RunMod: remove 'da_state_update' subroutine

This subroutine is inside an 'ICE_DA' CPP, which is not used in
any configuration. Remove it.

* drivers/hadgem3/CICE_RunMod: remove stray '+'

This '+' sign was copy-pasted there in error in 29b99b6 (CICE: Floe size
distribution  (CICE-Consortium#382), 2019-12-07). Remove it.

* drivers/hadgem3: remove obsolete 'check_finished_file' subroutine

Remove the call to 'check_finished_file' as well as the definition
of the subroutine, as the 'hadgem3' driver is not used on machine 'bering'
and it's unclear if machine 'bering' still exists.

* drivers/hadgem3: fix cice_init so it calls 'count_tracers'

This was forgotten back in 8b0ae03 (Refactor tracer initialization (CICE-Consortium#235), 2018-11-16)

* drivers/hadgem3/CICE_RunMod: add call to 'save_init'

The hadgem3 driver was not updated when 'save_init' was added in 83686a3
(Implement box model test from 2001 JCP paper (CICE-Consortium#151), 2018-10-22). As
this subroutine is necessary to ensure proper initialization of the
model, add it now.

* drivers/hadgem3/CICE_RunMod: tweak loop indices in 'coupling_prep'

Other drivers use 'ilo,ihi' and 'jlo,jhi' here. Do the same.

* drivers/hagdem3: update driver to new time manager

* drivers/hadgem3: pass 'mpi_comm_opa' explicitely to init_communicate

In 066070e (Fix minor issues in documentation, key_ CPPs, bfbcomp return
codes (#532), 2020-11-23), the 'ice_communicate' module was updated to
remove CPP macros relating to the OASIS coupler (key_oasis*) and to the
NEMO ocean model (key_iomput). These CPPs were used to make the correct
MPI communicator accessible to the 'init_communicate' subroutine.
However, that subroutine already accepts an optional MPI communicator as
argument and it was deemed cleaner to require the driver layer to
explicitely pass the communicator instead of making it accessible
through 'use' statements.

Update the 'hadgem3' driver, used for coupling with NEMO, to explicitely
pass the NEMO MPI communicator 'mpi_comm_opa' to 'init_communicate'.

* drivers: add 'nemo_concepts' driver

Historically the 'hadgem3' driver has been used when compiling a single
NEMO-CICE executable at ECCC.

Going forward, all driver-level adjustements will be done in our own
driver, 'nemo_concepts', 'CONCEPTS' being the name of the
multi-departmental collaboration around using the NEMO ocean model.

Copy CICE_InitMod, CICE_RunMod and CICE_FinalMod from the 'hadgem3'
directory to a new 'nemo_concepts' directory under 'drivers/direct'.

The following commits will clean up this new driver and port over some
in-house adjustments.

* drivers/nemo_concepts: remove unused 'writeout_finished_file' subroutine

This subroutine was only called on machine 'bering', which is not an
ECCC machine and probably does not exist anymore anyway. Remove it.

* drivers/nemo_concepts: call 'scale_fluxes' with 'aice_init'

Since 'merge_fluxes' is called with aice_init, it is more consistent to
also call 'scale_fluxes', in 'coupling_prep' with 'aice_init'
instead of 'aice'.

Copy this in-house change to the new 'nemo_concepts' driver.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eclare108213 eclare108213 eclare108213 approved these changes

@dabail10 dabail10 Awaiting requested review from dabail10

@daveh150 daveh150 Awaiting requested review from daveh150

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@apcraig @eclare108213