Bugfix for auto transforms

[dustinswales]

Description

Modifications made to handle local variables needed for variables transforms at the Scheme level. Previously the Group would manage the local variable

Fixes #594

In response to the scenarios in #594:

Scenario 1

If a variable is used in two different schemes and:

1. The first scheme has the same units as the host variable.
2. The second scheme has different units than the host variable.

The Error was replicated and fixed by removing a bugfix at line 941 in var_props.py.

Scenario 2

On the flip side, if:

1. The first scheme has different units than the host variable.
2. The second scheme has the same units as the host variable.

Output

Generated code for scenario number 2 (where the correct units are used):

! Compute reverse (pre-scheme) transforms
ps_local(:) = 1.0E-2_kind_phys*ps(:)
! Call scheme
call cld_liq_run(ncol=ncol, timestep=timestep, tcld=tcld, temp=temp, qv=qv,              &
              ps=ps_local, cld_liq_array=cld_liq_array, errmsg=errmsg, errflg=errflg)
...
! Call scheme
call cld_ice_run(ncol=ncol, timestep=timestep, temp=temp, qv=qv, ps=ps,            &
              cld_ice_array=cld_ice_array, errmsg=errmsg, errflg=errflg)

Testing

All Capgen tests pass.

[climbfuji]

Can you add a test for this, please? Similar to the one I added for prebuild?

[climbfuji]

Is sub_lname_list a somewhat ordinary list in Python? If so, then

if sub_lname_list:

is equivalent to line 165

[dustinswales]

@climbfuji Thanks for the review. I will expand the var_compatibility_test to stress more of the edge cases that @peverwhee brought up.

[dustinswales]

I think this was a bug. This needed to be need to BEFORE the allocation.

[peverwhee]

thanks for tackling this @dustinswales - this seems like it was a whole rabbit hole! 🐰

a couple small things and a testing question

[dustinswales]

@peverwhee It turns out that the real problem was in add_variable, where it was catching an incompatibility when it was actually equivalent. All good now, and unit tests updated to reflect the distinction between VarCompatObj.compat and VarCompatObj.__equiv

[peverwhee]

thanks @dustinswales !

[climbfuji]

approved

[climbfuji]

@gold2718 Would you like to review this PR?

[climbfuji]

Just typos, otherwise approved!

[dustinswales]

Just typos, otherwise approved!

Personally, I think it's more informative if Scheme, Group, Host, Var , etc..., which are (case sensitive) python classes, be referenced in their native form when commenting on them. Happy to change if others disagree, but "Group" is distinct from "group" in this context.

[climbfuji]

Just typos, otherwise approved!

Personally, I think it's more informative if Scheme, Group, Host, Var , etc..., which are (case sensitive) python classes, be referenced in their native form when commenting on them. Happy to change if others disagree, but "Group" is distinct from "group" in this context.

Ok, that's fine with me. The way I read the comments they looked like generic labels (a scheme, a group, ...) to me. Then let's just fix the type for compatibility please.

[mkavulich]

@dustinswales The other two PRs have been merged, let me know when this one is updated with the latest changes.

[dustinswales]

@mkavulich Updated.

[gold2718]

gold2718

Sorry, been out of commission. Trying to catch up.