Refactored evp sub cycling loop

[TillRasmussen]

For detailed information about submitting Pull Requests (PRs) to the CICE-Consortium,
please refer to: https://github.com/CICE-Consortium/About-Us/wiki/Resource-Index#information-for-developers

PR checklist

• Refactored the evp subcycling in order to cycle for each grid (B, C and CD)
  ENTER INFORMATION HERE

• [] Developer(s): @TillRasmussen
  ENTER INFORMATION HERE

• Suggest PR reviewers from list in the column to the right. @apcraig @eclare108213 @phil-blain

• Please copy the PR test results link or provide a summary of testing completed below.

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

  • [x ] bit for bit
  • different at roundoff level
  • more substantial

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

  • Yes
  • [ x] No

• Does this PR add any new test cases?
  refactorbaseCD.log
  refactorbasetB.log
  refactorbasetC.log
  refactortestB.log
  refactortestC.log
  refactortestCD.log

  • Yes
  • [ x] 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/. A test build of the technical docs will be performed as part of the PR testing.)

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

• Please provide any additional information or relevant details below:
  Test revealed that results after the change are bit for bit. There are test that failed while running
  1 test with dynpicard for B grid
  A number of test where namelist options for C and CD grid is not implemented.
  See attached files. base a original code. Test is new code for C, CD and B grids.
  This relate
  Test
  ./cice.setup --suite first_suite,base_suite,travis_suite,decomp_suite,reprosum_suite,quick_suite,omp_suite --mach freya --env intel,gnu --testid refactorbaseB --bgen refactorbaseB
  ./cice.setup --suite first_suite,base_suite,travis_suite,decomp_suite,reprosum_suite,quick_suite,omp_suite --mach freya --env intel,gnu --testid refactortestB --bcmp refactorbaseB
  ./cice.setup --suite first_suite,base_suite,travis_suite,decomp_suite,reprosum_suite,quick_suite,omp_suite --mach freya --env intel,gnu --testid refactorbaseC --bgen refactorbaseC -s gridc
  /cice.setup --suite first_suite,base_suite,travis_suite,decomp_suite,reprosum_suite,quick_suite,omp_suite --mach freya --env intel,gnu --testid refactortestC --bcmp refactorbaseC -s gridc
  /cice.setup --suite first_suite,base_suite,travis_suite,decomp_suite,reprosum_suite,quick_suite,omp_suite --mach freya --env intel,gnu --testid refactortestC --bcmp refactorbaseC -s gridc
  /cice.setup --suite first_suite,base_suite,travis_suite,decomp_suite,reprosum_suite,quick_suite,omp_suite --mach freya --env intel,gnu --testid refactortestC --bcmp refactorbaseC -s gridc

This relates to #755 where this is the bit for bit part and issue #739

[apcraig]

This looks fine to me. I think this may conflict with #755, but we'll have to see. I think I'll run a full test suite on cheyenne just to verify all is bit-for-bit independently. Then I think we should merge this before #755.

[TillRasmussen]

After @eclare108213 response to to #755 I decided to split the pull request in a binary identical and a none identical part. This being the identical part and #755 the none binary identical.

It is fairly simpel to handle a conflict with #755 afterwards.

[phil-blain]

There are test that failed while running
1 test with dynpicard for B grid

In refactorbasetB.log and refactortestB.log I see these dynpicard failures:

FAIL freya_intel_smoke_gx3_8x1_cmplogrest_dynpicard_reprosum_run10day_thread bfbcomp freya_intel_smoke_gx3_6x4_dynpicard_reprosum_run10day different-data
FAIL freya_gnu_smoke_gx3_8x1_cmplogrest_dynpicard_reprosum_run10day_thread bfbcomp freya_gnu_smoke_gx3_6x4_dynpicard_reprosum_run10day different-data

These are expected as the VP solver is not BFB between different decompositions currently. I'm finishing up my PR that solves this.

Do we understand all the other failures?

[phil-blain]

Thanks Till. I agree that this is good change to make in the current state of things.

[phil-blain]

same alignment fix needed here (I can't do a "Suggested change" since some lines are in the context).

[phil-blain]

Not something new in this PR, but I'm noticing that all calls to dyn_haloUpdate in ice_dyn_evp::evp use this capitalization: dyn_HaloUpdate (with a capital H), whereas the subroutine is defined in ice_dyn_shared as dyn_haloUpdate (small h). Not that big of a deal but it could be nice to fix that .

[TillRasmussen]

which is preferred?

[phil-blain]

I'm not sure if we have a firm standard or not. but for now I think we should use the case that is used when defining the subroutine (dyn_haloUpdate).

[apcraig]

I'm not too worried about this, but happy to go the way @phil-blain suggests.

[eclare108213]

I agree with @phil-blain, it's better to be consistent to make grepping just a little easier. We aren't completely consistent now but can work toward it.

[phil-blain]

grep and git grep were exactly what I had in mind when mentioning it (in fact that's how I noticed!)

[apcraig]

I have confirmed that a full test suite on cheyenne is bit-for-bit for all tests with this change, https://github.com/CICE-Consortium/Test-Results/wiki/cice_by_hash_forks#d74aaac6fa8e7915a2cc7b69df57f84372df2bb9

I think this can be merged, would like @eclare108213 to have a look/review first if she has a chance.

[TillRasmussen]

indents and case should be fixed. I did not redo the test