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.

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

Dummy and unused variables. #180

Merged
merged 36 commits into from
Sep 22, 2018
Merged

Dummy and unused variables. #180

merged 36 commits into from
Sep 22, 2018

Conversation

dabail10
Copy link
Contributor

This is just the removal or commenting out of a number of dummy arguments and unused variables. I have in some comments some uninitialized variable changes (answer changing) that will come in a later tag. However, everything in this pull request is BFB with the NAG compiler. There is a lot here and I am happy to discuss these.

  • Developer(s): D. Bailey

  • Please suggest code Pull Request reviewers in the column at right.

  • Are the code changes bit for bit, different at roundoff level, or more substantial? BFB

  • Is the documentation being updated with this PR? (Y/N) N
    If not, does the documentation need to be updated separately at a later time? (Y/N) N
    Note: "Documentation" includes information on the wiki and .rst files in doc/source/,
    which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.

  • Other Relevant Details:

@dabail10
Copy link
Contributor Author

With the initialization part, I was holding these off for a separate tag. This relies on some Icepack changes too. I agree that commenting out the BGC variables is misleading, but I left them commented so when they are actually used, they can easily be added back.

@dabail10
Copy link
Contributor Author

For the NAG compiler I used the -nan flag. Intel is supposed to use -check uninit, but this did nothing for me.

Dave

@apcraig
Copy link
Contributor

apcraig commented Sep 4, 2018

There is one new failure on all compilers on conrad,

conrad_*_restart_gx3_16x2x5x10x20_drakeX2

It fails during initialization with the following error,

(abort_ice)ABORTED:
(abort_ice) error = ice_distributionDestroy: error deallocating blockGlobalID

and the traceback is

cice 00000000004CF44E ice_exit_mp_abort 65 ice_exit.F90
cice 000000000049DEBD ice_distribution_ 1029 ice_distribution.F90
cice 000000000049902A ice_distribution_ 97 ice_distribution.F90
cice 00000000004AD545 ice_domain_mp_ini 518 ice_domain.F90
cice 00000000005829D1 ice_grid_mp_init_ 220 ice_grid.F90
cice 0000000000403518 cice_initmod_mp_c 51 CICE_InitMod.F90
cice 0000000000400646 MAIN__ 44 CICE.F90

The rake is the only decomp that calls ice_distributionDestroy. Had a quick look and it looks like the checks on successful deallocation in that routine are new. Maybe we need to back off on that check.

@dabail10
Copy link
Contributor Author

dabail10 commented Sep 4, 2018

So, this is a concern. The idea is that these function calls return an error status, but nothing was done with them. I added error handling for this purpose. Doesn't this highlight an issue in the rake algorithm?

@apcraig
Copy link
Contributor

apcraig commented Sep 4, 2018

In this case, I think it just means there is a temporary distrb type created that is not necessarily used entirely. I think it just stores a subset of information temporarily. We could add an arbitrary allocate on the array in the data structure that is not currently used. Or we could remove the deallocate istat checks. Either is acceptable to me. The former is more robust, the latter is more flexible. I guess I vote for the former if I have to vote.

This was referenced Sep 11, 2018
@dabail10
Copy link
Contributor Author

Can we merge this yet?

@apcraig
Copy link
Contributor

apcraig commented Sep 21, 2018

@dabail10, we just need the istat checks in the distributionDestroy routine turned off. The others in the rake routine should/can remain. Also, please confirm that you tested the drakeX2 option in your testing. thanks.

@dabail10
Copy link
Contributor Author

Ok. What is the difference between a drakeX2 and a rakeX2?

Dave

@apcraig
Copy link
Contributor

apcraig commented Sep 21, 2018

drakeX2 is the test option and will appear in the testname. It turns on the rakeX2 option in namelist.

@dabail10
Copy link
Contributor Author

So, the base_suite does a rakeX2 in the cheyenne_intel_decomp_gx3_4x2x25x29x5 test. Is this enough?

Dave

@apcraig
Copy link
Contributor

apcraig commented Sep 21, 2018

I believe a rakeX2 test in the decomp test is adequate. thanks.

Copy link
Contributor

@apcraig apcraig left a comment

Choose a reason for hiding this comment

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

I retested on 4 compilers and things looks OK. There is a seg fault in conrad_gnu_restart_gx3_6x2x50x58x1_droundrobin that I can't explain just now, but I am going to let that pass for the moment.

@apcraig apcraig merged commit 10c446a into CICE-Consortium:master Sep 22, 2018
mhrib pushed a commit to mhrib/CICE that referenced this pull request Sep 24, 2018
* update pgi compiler optimization to address reproducibility problems

* Dummy and unused variables. (CICE-Consortium#180)

* Remove some dummy arguments and unused variables.

* Dummy variable cleanup

* Cleanup

* Remove icepack from commit

* Reset icepack master

* Fix

* Fix

* Update icepack

* fix

* Fix flush ifdef logic

* Uninitialized variables.

* Additional initialization

* Update dummy

* more dummy fixes

* More hobart changes

* Update Hobart Intel settings

* Additional Hobart Intel changes

* Update icepack

* Some tweaks to dummy variables

* Remove some initialization code.

* Only remove istat checks in one routine.

* update icepack (CICE-Consortium#188)
@dabail10 dabail10 deleted the dummy2 branch September 27, 2018 15:41
apcraig pushed a commit that referenced this pull request Sep 28, 2018
* Allocatable dynamics version using namelist

* Updated ice_in according to "Allocated Dynamics"

* update scripts to automatically set namelist associated with grids and blocks

* fix uarea initialization related to NYGLOB cpp (#1)

* update pgi optimzation to address reproducibility problems (#2)

* mods for nag compiler failure (#3)

* Update to Consortium master bbec5d1 (#4)

* update pgi compiler optimization to address reproducibility problems

* Dummy and unused variables. (#180)

* Remove some dummy arguments and unused variables.

* Dummy variable cleanup

* Cleanup

* Remove icepack from commit

* Reset icepack master

* Fix

* Fix

* Update icepack

* fix

* Fix flush ifdef logic

* Uninitialized variables.

* Additional initialization

* Update dummy

* more dummy fixes

* More hobart changes

* Update Hobart Intel settings

* Additional Hobart Intel changes

* Update icepack

* Some tweaks to dummy variables

* Remove some initialization code.

* Only remove istat checks in one routine.

* update icepack (#188)

* fix ice_abort error in ice_state.F90 (#5)
phil-blain added a commit to phil-blain/CICE that referenced this pull request Sep 30, 2020
In subroutine 'makemask', the array 'uvm' is initialized in a loop on indices
{ilo, ihi} and {jlo, jhi}, but is then used in a loop on indices {1, nx_block} and
{1, ny_block}, causing an uninitialized value to be used (at line 1672).

Initialize the whole array to zero first, to avoid using uninitialized values.
The initialization to zero is actually commented out, following
CICE-Consortium#180, so uncomment it.
phil-blain added a commit to phil-blain/CICE that referenced this pull request Sep 30, 2020
In subroutine 'init_grid2', the array 'tarea' is initialized in a loop on
indices {ilo, ihi} and {jlo, jhi}, but is then used in subroutine 'makemask' in
a loop on indices {1, nx_block} and {1, ny_block}, causing an uninitialized
value to be used (at line 1693).

Initialize the whole array to zero first, to avoid using uninitialized values.
The initialization to zero is actually commented out, following
CICE-Consortium#180, so uncomment it.
phil-blain added a commit to phil-blain/CICE that referenced this pull request Oct 20, 2020
In subroutine 'init_grid2', the array 'tarea' is initialized in a loop on
indices {ilo, ihi} and {jlo, jhi}, but is then used in subroutine 'makemask' in
a loop on indices {1, nx_block} and {1, ny_block}, causing an uninitialized
value to be used (at line 1693).

Initialize the whole array to zero first, to avoid using uninitialized values.
The initialization to zero is actually commented out, following
CICE-Consortium#180, so uncomment it.

(cherry picked from commit ca2f4d2)
phil-blain added a commit to phil-blain/CICE that referenced this pull request Oct 20, 2020
In subroutine 'makemask', the array 'uvm' is initialized in a loop on indices
{ilo, ihi} and {jlo, jhi}, but is then used in a loop on indices {1, nx_block} and
{1, ny_block}, causing an uninitialized value to be used (at line 1672).

Initialize the whole array to zero first, to avoid using uninitialized values.
The initialization to zero is actually commented out, following
CICE-Consortium#180, so uncomment it.

(cherry picked from commit dacea48)
phil-blain added a commit to phil-blain/CICE that referenced this pull request Dec 17, 2020
In subroutine 'makemask', the array 'uvm' is initialized in a loop on indices
{ilo, ihi} and {jlo, jhi}, but is then used in a loop on indices {1, nx_block} and
{1, ny_block}, causing an uninitialized value to be used (at line 1672).

Initialize the whole array to zero first, to avoid using uninitialized values.
The initialization to zero is actually commented out, following
CICE-Consortium#180, so uncomment it.
phil-blain added a commit to phil-blain/CICE that referenced this pull request Dec 17, 2020
In subroutine 'init_grid2', the array 'tarea' is initialized in a loop on
indices {ilo, ihi} and {jlo, jhi}, but is then used in subroutine 'makemask' in
a loop on indices {1, nx_block} and {1, ny_block}, causing an uninitialized
value to be used (at line 1693).

Initialize the whole array to zero first, to avoid using uninitialized values.
The initialization to zero is actually commented out, following
CICE-Consortium#180, so uncomment it.
phil-blain added a commit to phil-blain/CICE that referenced this pull request Jan 28, 2021
In subroutine 'init_grid2', the array 'tarea' is initialized in a loop
on indices {ilo, ihi} and {jlo, jhi}, but is then used in subroutine
'makemask' in a loop on indices {1, nx_block} and {1, ny_block},
potentially causing an uninitialized value to be used (at line 1693).

Initialize the whole array to zero first, to avoid using uninitialized values.
The initialization to zero is actually commented out, following
10c446a (Dummy and unused variables. (CICE-Consortium#180), 2018-09-22) [1] , so
uncomment it.

[1] CICE-Consortium#180
phil-blain added a commit to phil-blain/CICE that referenced this pull request Jan 28, 2021
In subroutine 'makemask', the array 'uvm' is initialized in a loop on indices
{ilo, ihi} and {jlo, jhi}, but is then used in a loop on indices {1, nx_block} and
{1, ny_block}, potentially causing an uninitialized value to be used (at line 1672).

Initialize the whole array to zero first, to avoid using uninitialized values.
The initialization to zero is actually commented out, following
10c446a (Dummy and unused variables. (CICE-Consortium#180), 2018-09-22) [1], so uncomment it.

[1] CICE-Consortium#180
apcraig pushed a commit to apcraig/CICE that referenced this pull request Mar 24, 2021
In subroutine 'init_grid2', the array 'tarea' is initialized in a loop
on indices {ilo, ihi} and {jlo, jhi}, but is then used in subroutine
'makemask' in a loop on indices {1, nx_block} and {1, ny_block},
potentially causing an uninitialized value to be used (at line 1693).

Initialize the whole array to zero first, to avoid using uninitialized values.
The initialization to zero is actually commented out, following
10c446a (Dummy and unused variables. (CICE-Consortium#180), 2018-09-22) [1] , so
uncomment it.

[1] CICE-Consortium#180
apcraig pushed a commit to apcraig/CICE that referenced this pull request Mar 24, 2021
In subroutine 'makemask', the array 'uvm' is initialized in a loop on indices
{ilo, ihi} and {jlo, jhi}, but is then used in a loop on indices {1, nx_block} and
{1, ny_block}, potentially causing an uninitialized value to be used (at line 1672).

Initialize the whole array to zero first, to avoid using uninitialized values.
The initialization to zero is actually commented out, following
10c446a (Dummy and unused variables. (CICE-Consortium#180), 2018-09-22) [1], so uncomment it.

[1] CICE-Consortium#180
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants