What should be the order of overridden environment variables and other runtime settings?

[TomekTrzeciak]

Describe the bug

Based on documentation (emphasis mine):

Values [of environment variables] may refer to other local environment variables (order of definition is preserved)

Cylc uses OrderedDict to store configuration items in their definition order. However, when combined with family inheritance the resulting order can be surprising (from the the user perspective), i.e., the order of local definitions can be violated.

Steps to reproduce the bug

Consider the following example:

[scheduling]
   [[dependencies]]
       graph = task
[runtime]
    [[FAM]]
        [[[environment]]]
            BAR = bar
    [[task]]
        inherit = FAM
        [[[environment]]]
            FOO = foo
            BAR = ${FOO}

This will store environment of task as:

$ cylc get-config --sparse -i '[runtime][task][environment]' SUITENAME
BAR = ${FOO}
FOO = foo

Note that BAR variable is now before FOO, unlike in the user definition, which will lead to unbound variable error in the job script. The reason for this is the definition of BAR under FAM. Even though BAR is subsequently redefined under task, the order of variables is determined by their first assignment rather than the last.

So, if BAR is not defined under FAM, the suite works OK. Then, if at a later time one adds BAR under FAM (because it might be needed by some other task(s) inheriting from FAM), the suite suddenly breaks and it's not very intuitive why.

Expected behavior

I think the most intuitive (and sound) behaviour is to preserve the order of local definition under the environment section, i.e., follow the override order rather than the order of first definition, which might be somewhere deep within the inheritance hierarchy.

The fix is relatively trivial and was proposed in cylc/cylc-flow#3305 but rejected as unsuitable for a patch release.

Additional context

@hjoliver considers (cylc/cylc-flow#3305 (comment)) a variable to be defined by its (oldest) parent, not when it is overridden. I think that interpretation is quite debatable. I would argue that, when overridden, the variable is simply redefined, i.e., its definition changes. (In formal terms, binding a symbol (name) to a value forms its definition and thus rebinding the symbol to a different value redefines it.)

[sadielbartholomew]

I'm not convinced the 'bug' label is appropriate here, in cylc/cylc-flow#3305 it was discussed & from the comments seemed to emerge as a subjective issue relating to the most appropriate behaviour.

[oliver-sanders]

Solutions I can think of:

• Use a bash parser to pick out variable names and order accordingly

  for an outline of what's possible see https://vorpaljs.github.io/bash-parser-playground/

• Don't override environment variables, dump both sections in namespace order i.e:

   # FAM
   BAR='bar'
   # task
   FOO='foo'
   BAR="${FOO}"
  

[TomekTrzeciak]

Other solutions:

• Follow documentation and preserve local definition order
• Change documentation to match current implementation

[hjoliver]

I've tentatively put this against Cylc 8; shouldn't be too hard to agree a solution and implement.

[oliver-sanders]

Leaning towards the documentation option as any change is likely to break workflows. Needs further investigation and transfer to cylc-doc.

[TomekTrzeciak]

Leaning towards the documentation option

😞

as any change is likely to break workflows.

I wouldn't expect too many suites to depend on that behaviour (and whatever breakage happens to be, should be a small fry and easy to fix compared to what SoD will bring), but then there's no easy way to test my theory.

[hjoliver]

Coming back to review this at last. Apologies for the long delay @TomekTrzeciak ... unfortunately you raised it just before Cylc 8 development got under way!

On review, and considering comments on the parent issue as well, I think we will stick with the status quo, but document it properly. Reasons:

• there are legit concerns about breaking existing workflows by changing this
• and (importantly) there's not a single unarguable right answer; it could be confusing to users either way

What to document for users, then:

• If you both set and reference the value of a task environment variable in your workflow config, you need to be aware of definition order, because a variable can't be used before it is defined
• The definition order in the job script is determined by the first appearance of a variable definition in:
  a) primarily, family inheritance order (if a variable is set at multiple levels of the hierarchy)
  b) secondarily, order within a single environment section
• If a variable gets redefined, the value gets overridden but the definition order does not change

Further justification (bearing in mind you can make an argument both ways):

• This results in consistent behaviour regardless of the order of runtime sections
• It makes sense that definitions in families take precedence over inheriting families and tasks

[hjoliver]

Can be closed by a documentation PR. (Which I'm not pushing onto @TomekTrzeciak 😁 )