parameters: complete support for alternative syntax

[oliver-sanders]

[edit] Read down, this syntax was not intended to work, but does work in all other contexts.

Cylc allows us to inherit from parametrised families:

[runtime]
    [[X<foo>]]
    [[x<foo>]]
        inherit = X<foo>

However, for some reason this only works when there is a single parameter involved, this example errors:

# suite.rc
[cylc]
    [[parameters]]
        foo = 1..2
        bar = 3..4

[scheduling]
    [[dependencies]]
        [[[R1]]]
            graph = x<foo><bar>

[runtime]
    [[X<foo><bar>]]
    [[x<foo><bar>]]
        inherit = X<foo><bar>

Reported with Cylc 7.

Pull requests welcome!
This is an Open Source project - please consider contributing a bug fix
yourself (please read CONTRIBUTING.md before starting any work though).

[hjoliver]

I think we can close this as invalid, possibly after adding a warning to the documentation and improved validation.

foo<m><n> is not documented as valid syntax and I'm surprised it works even without inheritance.

It should be foo<m,n>, and that does work with inheritance:

[task parameters]
    m = 1,2
    n = a,b
[scheduling]
    [[graph]]
        R1 = "FOO<m,n>"
[runtime]
    [[FOO<m,n>]]
    [[foo<m,n>]]
        inherit = "FOO<m,n>"

[oliver-sanders]

foo is not documented as valid syntax

And yet it works fine in [runtime] sections. Given this I expected it to work elsewhere.

In this particular example there is a semi-legit use case for separating the two:

   abc_<foo>_def_ghi_<bar>

[oliver-sanders]

I ran a quick scan for <.*>.*<.*> and found this syntax is fairly widely used, here are some examples I found:

run_pointstat_1<model><fcstr_pt> => check_metlogs_pointstat<model>
plot_fields_mogrepsuk<fieldtypemogrepsuk><fieldgridmogrepsuk>
visualise<model><diagnostic=tempdaymax><step><forecast_representation=percentiles>
get_file_names<model><diagnostic><step>
plot_analysis_UKV<UKVanalyses><analysisgridUKV>
run_gridstat_1<model><fcstr_gd_1>
fluxes_interp<fluxmode><member=0>
permute_ks<streams><permute>

All of these could have been implemented as <a, b>, but weren't.

Scanning for <.*,.*> I also found examples of the correct syntax:

pollerStaticLeads<suite=oper, model=ukv, runtime=03>_660mins
imagegen<suite=oper, model=ukv, polar_sat, band=vis, runtime=00>
getTest<filePref, inputFile-5> => getTest<filePref, inputFile>
gshc_recon_m<gshc_mem> => gshc_model_m<gshc_mem, gshc_step=1>

I think the <a><b> is a natural pattern, people give it a try, it works so they stick with it.

[hjoliver]

Yeah that pattern is equally natural, but we made a choice between several possible syntaxes back when originally implemented. Supporting multiple syntaxes for almost the same purpose would be a bit unusual.

But we could certainly support that syntax as well, in principle, of course.

It doesn't strike me as a high priority issue though. The ability to spread parameter task name components out a bit might be nice sometimes, but I doubt that its ever critical, esp. as we can at least control the order of the parameters.

If it's easy to implement though, why not. It might well be easy given that it damn near works already!

[hjoliver]

(It is interesting that a fair number of people are evidently using the "wrong" syntax though!)

[oliver-sanders]

My scan's hanging but I wouldn't be surprised if the "wrong" usage outweighs the "correct" usage. Just feels more natural.

[oliver-sanders]

Ok, we have a workaround, pushing this back.

I think we need to address this as we currently support two syntaxes, but only one of them globally, which is highly confusing.

TBH, I think <foo,bar> feels like shorthand for <foo><bar>, the condensed syntax definitely wouldn't have been my first guess if I was new to parameters.

Given that we've been supporting <foo,bar> in all other contexts for the past few years I think it's legitimate to expect it to work in inherit statements (which must be going through a different interface for some reason?).

[hjoliver]

I think we need to address this as we currently support two syntaxes, but only one of them globally, which is highly confusing.

"currently support two syntaxes" is a bit of an overstatement!

We have one supported and documented syntax, and another that some users have guessed at and which by some strange miracle happens to both work (in most but not all cases) and to not be actively disallowed.

However, given that, I'm fine with formalizing the accidental support and extending it to make inheritance work.

[hjoliver]

inherit statements (which must be going through a different interface for some reason?).

That's done by NameExpander.expand_parent_names(). It's a different interface from name expansion in the graph and in runtime namespace headings, because here we need to replace the parameter syntax with the specific parameter values from the already-expanded enclosing namespace rather than expanding over all parameter values.

It doesn't work for this erm, alternative syntax, because the method (which is called once for each parent name) assumes all the parameters are contained within a single <...> group.

Probably not difficult to make it work.

[oliver-sanders]

🛏️