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

Unused and Dummy variables #142

Merged
merged 14 commits into from
Feb 8, 2018
Merged

Unused and Dummy variables #142

merged 14 commits into from
Feb 8, 2018

Conversation

dabail10
Copy link
Contributor

@dabail10 dabail10 commented Feb 8, 2018

This is a big one as I have removed all of the dummy and unused variables. I have attempted to just comment out cases that look like place holders. Note there are two answer changing bugs here. One is in icepack_atmo.F90 and icedrv_init_column.F90. These are issues #136 and #138.
Developer(s): D. Bailey
Are the code changes bit for bit, different at roundoff level, or more substantial? The changes are more than roundoff, but should not change the climate.
Is the documentation being updated with this PR? (Y/N) N
If not, does the documentation need to be updated separately? (Y/N) N
"Documentation" includes information on the wiki and .rst files in doc/source/,
which are used to create the online technical docs at https://cice-consortium.github.io/Icepack/
Please suggest code reviewers in the column at right.
Other Relevant Details:

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.

I think the SVN:$Id: lines should be removed completely, since the dates, versions numbers etc will no longer change with new code modifications. This doesn’t have to be done as part of this PR.

Another note: Preprocessing flag changes from CCSMCOUPLED to CESMCOUPLED will also need to be done in CICE.

in subroutine flush_pond (icepack_therm_mushy), I think you should leave both of the 'sadded' lines in the code, but commented out, for later reference. We might need to look back at what's done and that information could be helpful.

In subroutine ice_step (icedrv_RunMod), please make a line for debugging and comment it out like this (this is useful for debugging, using other commented-out lines below):
! use icedrv_diagnostics, only: icedrv_diagnostics_debug

@dabail10
Copy link
Contributor Author

dabail10 commented Feb 8, 2018

I added your suggested changes and went ahead and got rid of the SVN lines. I agree that CICE needs CCSMCOUPLED changes to CESMCOUPLED as well.

Dave

@dabail10
Copy link
Contributor Author

dabail10 commented Feb 8, 2018

Don't approve yet, I have more to come.

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.

Thanks Dave, I'm sure this was a bunch of work.

@apcraig
Copy link
Contributor

apcraig commented Feb 8, 2018

Dave, is this ready to merge once we get reviews done? Or do you have more?

@eclare108213 eclare108213 merged commit 9262d7e into CICE-Consortium:master Feb 8, 2018
@eclare108213 eclare108213 mentioned this pull request Jul 11, 2022
lettie-roach pushed a commit to lettie-roach/Icepack that referenced this pull request Oct 18, 2022
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