Fix perfmetrics v2.8.0

[axel-lauer]

This PR fixes a newly surfaced issue with the perfmetrics diagnostics "zonal", "latlot" and "cycle" that resulted in the diagnostics getting stuck in an infinite loop (see also #3076).

As it turned out, the problem was local variables used in functions defined in interface_scipts/auxiliary.ncl that had the same variable name as a loop variable in the perfmetrics diagnostics mentioned above. For reasons that are still not clear to me, a call to the function ncdf_write (which then calls ncdf_define) resulted in the variable used as loop index (ii) being overwritten in the calling procedure. As a consequence, the calling diagnostic got stuck in an infinite loop.

Declaring the variables used within the shared functions (interface_scipts/auxiliary.ncl) as "local" solved this problem.

I still do not fully understand why variables used in a function would overwrite the variables with the same name in the calling procedure when not explicitly declared as "local" variables and why this problem did not emerge earlier, but this problem is fixed with this PR. With the fix, I could (again) successfully run recipe_perfmetrics_CMIP5.yml, the output looks as expected.

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.

• 🛠 This pull request has a descriptive title
• 🛠 Code is written according to the code quality guidelines
• 🛠 Documentation is available
• 🛠 Tests run successfully
• 🛠 The list of authors is up to date
  - [ ] 🛠 Any changed dependencies have been added or removed correctly
• 🛠 All checks below this pull request were successful

New or updated recipe/diagnostic

• 🧪 Recipe runs successfully
  - [ ] 🧪 Recipe is well documented
• 🧪 Figure(s) and data look as expected from literature
• 🛠 Provenance information has been added

[valeriupredoi]

great detective work @axel-lauer 🔍 Could this perhaps be a exposing a bug in the latest build of NCL? 6.6.2 it's ben since ages, since no development being done on it, but the latest build is about two weeks old, it may be that that's the culprit?

[LisaBock]

Thanks @axel-lauer !
Worked fine for me as well. But I think it would be enough to define ii as local variable in interface_scripts/auxiliary.ncl.
A reason for this problem could be that there is ii used in diag_scripts/perfmetrics/main.ncl since my PR #2856 and therefore it is defined as a global variable for all appending funtions. And this is the reason why it is now passed from interface_scripts/auxiliary.ncl to the perfmetrics diagnostics "zonal", "latlot" and "cycle". Just an idea...

[axel-lauer]

Thanks @LisaBock for testing. It sounds quite possible, that the changes to main.ncl from #2856 could be the reason for this problem.
Declaring also the variables i, jj and idim explicitly as local doesn't hurt as it makes the code (in my opinion) a bit easier to read, so I would tend to leave that in.

[remi-kazeroni]

Thanks a lot for your work on this @axel-lauer! This fix enabled me to run the 4 affected recipes successfully (recipe_perfmetrics_CMIP5_* and recipe_smip_*) 👍
This is most probably related to a previous PR that has introduced a conflict in the diagnostics, not something related to the NCL package itself. quite a few NCL diagnostics have been added or changed since the last release. Good to see these all run fine with this fix.

[valeriupredoi]

fantastic work, guys and gals! Am merging this now 🍺

[valeriupredoi]

boom!