Clean unused var

[jm-c]

What changes does this PR introduce?

Remove or avoid un-used (local) variables in (TAF) AD built and clean-up (a little) pkg/smooth.

What is the current behaviour?

Most un-used variables have been removed from FWD verification experiment built but still many seen in AD verification exp. built (although the number went down by > x 31 after merging PR #471).

What is the new behaviour

clean up by removing or just avoiding some un-used variables

Does this PR introduce a breaking change?

no real code change, zero change in results.

Other information:

a) for now, no changes in pkg/streamice (left AD exp halfpipe_streamice on the side).
b) there is still unused variables left in src files that are processed by TAF, but, since TAF remove them, it's less an issue + don't notice any warning.

Suggested addition to tag-index

o pkgs:

• remove/avoid unused variables from AD verification exp. built except from
  halfpipe_streamice (pkg/streamice); also clean-up a bit pkg/smooth.

[jm-c]

Some stats: for all AD verification exp, the number of used variable warnings in all built when down from 31,700 before PR #471 to 1062 with current master, and now down to 40 with this PR (including 38 from halfpipe_streamice built).

Also will remove few more unused var that don't show up in AD built log file because TAF remove them.

[mjlosch]

@jm-c just a comment/suggestion: These variables defined as parameters in tamc.h:

      integer nyears_chkpt
      integer nmonths_chkpt
      integer ndays_chkpt
      integer ngeom_chkpt
      integer ncheck_chkpt

are not used anywhere in the code, as far as I can see. They do not produce unused-warning messages, because of the parameter statements, but they are unused, nevertheless. Maybe we can remove them in the PR as well?

[jm-c]

@mjlosch Thaks for the sugestion. I will remove these unsed "parameters"

[mjlosch]

Not yet a review, but just a comment: in the context of #529 I already removed a few unused variables from pkg/streamice. Further I noticed that "ECCO_CPPOPTIONS.h" is still being used in "global_ocean_ebm" and "halfpipe_streamice". I cannot remember why. I have started a branch to fix that, and this may be yet another chance to clean up some TAF-related aspects of streamice (by no means everything). I will wait for this and #529 to be merged before proceeding and turning that branch into a PR.

[mjlosch]

This is a very useful (and tedidious) clean-up PR. Obviously, I did not check all 96 files. With

grep -B4 "Warning: Unused variable" */build/make.tr_log

I found only 39 warnings, all but one in halfpipe_streamice. The only other one in bottom_ctrl_5x5, which could be suppressed by a cpp-flag in ctrl_unpack.F (in analogy to ctrl_pack.F):

#if (defined ALLOW_OBCS || defined ECCO_CTRL_DEPRECATED)
      character*( 80) weighttype
#endif

Should be we add that? Otherwise, LGTM and should be merged quickly, because it affects so many files and hence current PRs, for which we need to make sure, that they don't break this cleanup. The earlier we do this, the better.

[jm-c]

@mjlosch Thanks for the review. I fixed ctrl_unpack.F the way you suggested (I probably missed this one in the first round of changes).
Regarding un-used parameters in tamc.h, it seems better (from my point of view) to do this in an other PR, since:
a) this would require to update all tamc.h from all AD verification exp. code_ad, since the reference version in pkg/autodiff is never used. So probably better to do this in a PR focused on verfication//code_ad cleaning.
b) the only "_chkpt" that is used, i.e., nthreads_chkpt, could also be removed from tamc.h and from few src files,
at least as I see it, because it should never be different from unity. So maybe something to sort out at the same time as removing all the others.

[jm-c]

And regarding this:

Not yet a review, but just a comment: in the context of #529 I already removed a few unused variables from pkg/streamice. Further I noticed that "ECCO_CPPOPTIONS.h" is still being used in "global_ocean_ebm" and "halfpipe_streamice". I cannot remember why. I have started a branch to fix that, and this may be yet another chance to clean up some TAF-related aspects of streamice (by no means everything). I will wait for this and #529 to be merged before proceeding and turning that branch into a PR.

I avoided changing anything in pkg/streamice so no risk of conflicts. And regarding CPP header files from verification code_ad dir I also noticed #577 which could be cleaned-up as well.

[mjlosch]

@jm-c it's not clear to me why weighttype is commented out in "ctrl_unpack.F" once for the generic controls (2D)

MITgcm/pkg/ctrl/ctrl_unpack.F

Lines 895 to 896 in 3b6c79e

	cc write(weighttype(1:80),'(80a)') ' '
	cc write(weighttype(1:80),'(a)') "wunit"

but three times in "ctrl_pack.F". To me this appears totally inconsistent. Plus, weighttype is not used in any of the generic controls, as far as I can see. So instead of 28566e3 we could remove (or comment out) weighttype consistently with "ctrl_pack.F". Maybe we can have an opinion from the ECCO crowd (@owang01 , @ifenty).

[jm-c]

@mjlosch I will follow your suggestion and go back 1 commit (to d1cea20 ) and comment out these internal "write". Since these 'write" do not do anything and are commented out in 1/3 places in this routine and all 3/3 in ctrl_pack.F it's just cleaner to have also 3/3 commented out here.

[jm-c]

@mjlosch I agree that, since this PR involves many files, it would be easier if it could be merged relatively soon.
So if you agree with both latest changes + my comments, we could merge it in the coming days.

[mjlosch]

@jm-c your latest changes look good to me. Let's merge this.

[jm-c]

OK, unless someone disagree, will merge this PR tomorrow.